Improve worker deployment queries #1616
Conversation
…some of the columns are huge
|
|
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/presenters/v3/TestTaskPresenter.server.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. WalkthroughThe changes in the Changes
Poem
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 (
|
@trigger.dev/react-hooks
@trigger.dev/build
@trigger.dev/rsc
@trigger.dev/sdk
trigger.dev
@trigger.dev/core
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/v3/models/workerDeployment.server.ts (1)
Line range hint
112-137: Consider transitioning remaining queries to useselectFor consistency with the PR's objectives and other changes, consider updating these queries to use
selectinstead ofinclude. This would maintain the pattern of explicit field selection and potentially improve query performance.- include: { - deployment: true, - tasks: true, - }, + select: { + id: true, + friendlyId: true, + version: true, + sdkVersion: true, + cliVersion: true, + supportsLazyAttempts: true, + deployment: { + select: { + id: true, + imageReference: true, + version: true, + } + }, + tasks: { + select: { + id: true, + friendlyId: true, + slug: true, + filePath: true, + exportName: true, + triggerSource: true, + machineConfig: true, + maxDurationInSeconds: true, + } + }, + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts(2 hunks)apps/webapp/app/v3/models/workerDeployment.server.ts(5 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 (buildjet-8vcpu-ubuntu-2204 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (buildjet-8vcpu-ubuntu-2204 - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/webapp/app/v3/models/workerDeployment.server.ts (4)
11-22: Well-defined type for optimized task data retrieval!The
BackgroundWorkerTaskSlimtype effectively defines the minimal set of fields needed for task operations, which aligns well with the PR's goal of optimizing queries.
25-51: Excellent use of explicit field selection!The transition from
includetoselectwith explicit field selection will help Prisma generate more efficient queries, reducing unnecessary data fetching.
Line range hint
57-82: Good transition tofindFirstwith optimized field selection!The change from
findUniquetofindFirstalong with explicit field selection aligns perfectly with the PR's performance optimization goals.
89-92: Precise return type definition!The use of
Pickto specify exact fields improves type safety and documents the function's output contract clearly.apps/webapp/app/presenters/v3/TestTaskPresenter.server.ts (1)
6-9: Clean type integration!The integration of
BackgroundWorkerTaskSlimtype is well done, maintaining type safety while benefiting from the optimized field selection.Also applies to: 119-119
When dequeuing runs, and in a couple of other places, we get details about the current worker deployment.
These queries were using
findUniqueand includes, which is a VERY BAD thing to do with Prisma because it changes the queries it produces to very bad ones when run concurrently.Also we were selecting a lot of columns we don't need.
This PR changes to
findFirstand only selecting relevant columns, for big query performance improvements.Summary by CodeRabbit