Force upgrade v1 batches to v3 #1676
Conversation
|
WalkthroughThe changes update the API route for batch task requests with new request body transformations and an upgraded service. A conversion function is introduced to translate legacy request bodies to the new format, and error handling is enhanced. In the service layer, a dynamic batch processing size threshold is configured via the constructor. Additionally, the related schemas are restructured with new optional fields and reorganized properties to support these updates. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as API Endpoint
participant F as convertV1BodyToV2Body
participant S as BatchTriggerV3Service
participant DB as Database
C->>A: Send batch task request
A->>F: Convert v1 body to v2 format
F-->>A: Return transformed body
A->>S: Invoke service with transformed body and parameters
S->>S: Check asyncBatchProcessSizeThreshold condition
alt Inline processing
S->>DB: Process batch task inline
else Transactional processing
S->>DB: Process batch task via transaction
end
S-->>A: Return result (batchId & run ids)
A-->>C: Respond with updated response structure
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/webapp/app/routes/api.v1.tasks.$taskId.batch.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts (1)
132-156: Consider enhancing type safety in the conversion function.The conversion function is well-structured and properly documented. To enhance type safety, consider using the
Pickutility type to explicitly define which options should be retained rather than manually unsetting specific options.Here's a suggested improvement:
function convertV1BodyToV2Body( body: BatchTriggerTaskRequestBody, taskIdentifier: string ): BatchTriggerTaskV2RequestBody { return { items: body.items.map((item) => ({ task: taskIdentifier, payload: item.payload, context: item.context, options: item.options ? { - ...item.options, - dependentBatch: undefined, - parentBatch: undefined, - dependentAttempt: undefined, + ...Pick<typeof item.options, Exclude<keyof typeof item.options, 'dependentBatch' | 'parentBatch' | 'dependentAttempt'>>, } : undefined, })), dependentAttempt: body.dependentAttempt, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts (2)
95-102: LGTM! Well-structured request handling.The conversion from v1 to v2 body format and the service call are properly implemented with comprehensive error handling and context propagation.
88-88: Configure BatchTriggerV3Service with execution parameters.According to the AI summary, BatchTriggerV3Service accepts parameters for execution mode and maximum batch size in its constructor. Consider configuring these parameters during initialization.
Let's verify the constructor parameters:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for BatchTriggerV3Service constructor definition ast-grep --pattern $'class BatchTriggerV3Service { constructor($_) { $$$ } }'Length of output: 86
Let's gather more details about the BatchTriggerV3Service definition by searching for its class declaration and constructor using ripgrep. This will help us verify what parameters, if any, the constructor accepts.
🏁 Script executed:
#!/bin/bash # Search for the class declaration for BatchTriggerV3Service rg "class BatchTriggerV3Service" -n # Additionally, search for the constructor definition within files containing the class. rg "class BatchTriggerV3Service" -n | while read -r line; do filename=$(echo "$line" | cut -d: -f1) echo "Searching in $filename for constructor details..." rg "constructor(" "$filename" -n doneLength of output: 766
Let's gather more details from the service definition by re-running the search with escaped parentheses to handle the regex error. This will help us confirm if the constructor accepts execution parameters.
🏁 Script executed:
#!/bin/bash # Re-run regex search with escaped parentheses to avoid regex parse errors rg -A15 "constructor\\(" apps/webapp/app/v3/services/batchTriggerV3.server.tsLength of output: 675
Explicitly configure BatchTriggerV3Service with execution parameters.
Our investigation confirms that BatchTriggerV3Service’s constructor accepts parameters—specifically, an optional batchProcessingStrategy (which defaults to "parallel") and asyncBatchProcessSizeThreshold (defaulting to ASYNC_BATCH_PROCESS_SIZE_THRESHOLD)—in addition to a Prisma client that defaults to the prisma instance. Instantiating the service without arguments causes it to rely on these defaults. If the intent is to use custom execution parameters, explicitly configuring them during initialization would improve clarity and maintainability.
Summary by CodeRabbit