fix: default machine config indexing now works #1979
Conversation
🦋 Changeset detectedLatest commit: 85e8e30 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update introduces logic to apply a default machine preset from the global configuration to tasks that do not already specify a machine preset. The new behavior is implemented in both the development and managed index worker entry points. Additionally, the configuration and example files in the "hello-world" reference are updated to include explicit machine presets. A new changeset documents these adjustments as a patch for the "trigger.dev" component, specifically addressing default machine configuration indexing. Changes
Sequence Diagram(s)sequenceDiagram
participant Config
participant IndexWorker
participant Task
Config->>IndexWorker: Provide global config (may include machine preset)
IndexWorker->>Task: For each task, check if machine.preset is set
alt Task has no machine.preset
IndexWorker->>Task: Assign machine.preset from config
else Task already has machine.preset
IndexWorker->>Task: Leave machine.preset unchanged
end
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 0
🧹 Nitpick comments (2)
references/hello-world/trigger.config.ts (1)
18-18: Machine preset configuration looks good!The addition of the
machine: "large-1x"configuration sets a global default machine preset that will be applied to all tasks that don't specify their own machine preset.Consider adding a brief comment above this property to clarify its purpose:
+ // Default machine preset to use for all tasks that don't specify their own machine: "large-1x",packages/cli-v3/src/entryPoints/managed-index-worker.ts (1)
131-145: Default machine preset implementation looks solid.The implementation correctly applies the global machine preset to tasks that don't have one already defined, following the same pattern used for retry defaults and maxDuration.
Consider adding a comment to clarify the behavior when a task has a
machineobject but nopresetproperty:// If the config has a machine preset, we need to apply it to all tasks that don't have a machine preset if (typeof config.machine === "string") { tasks = tasks.map((task) => { + // This will also replace any existing machine config without a preset property if (typeof task.machine?.preset !== "string") { return { ...task, machine: { preset: config.machine, }, } satisfies TaskManifest; } return task; }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/spotty-ducks-punch.md(1 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts(1 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts(1 hunks)references/hello-world/src/trigger/example.ts(1 hunks)references/hello-world/trigger.config.ts(1 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: units / 🧪 Unit Tests
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.changeset/spotty-ducks-punch.md (1)
1-5: Changeset looks good.The changeset correctly documents this as a patch update for trigger.dev with an appropriate description of the fix.
references/hello-world/src/trigger/example.ts (1)
40-40: Task-level machine preset looks good.The addition of
machine: "medium-1x"to this task demonstrates how individual tasks can override the global machine preset defined in the config file ("large-1x").packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)
131-145: Looks good! This change correctly applies the default machine preset config to tasks.The added code follows the same pattern as the existing conditional blocks for handling retry and maxDuration defaults. It appropriately:
- Checks if a global machine preset is configured
- Only applies the default to tasks that don't already have a machine preset defined
- Preserves other task properties when applying the default
This implementation ensures consistent behavior across all tasks and completes the feature set for applying global defaults.
Summary by CodeRabbit
New Features
Bug Fixes