Conversation
Deprecate maxDuration in favour of maxComputeSeconds but keep backwards compatibility
🦋 Changeset detectedLatest commit: 58fbc71 The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 packages
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 |
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
WalkthroughThis pull request deprecates the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 triggerAndWait_internal not updated to use maxComputeSeconds
The triggerAndWait_internal function at packages/trigger-sdk/src/v3/shared.ts:2399 still passes options?.maxDuration directly, without the maxComputeSeconds fallback. Every other call site in the file was updated to options?.maxComputeSeconds ?? options?.maxDuration (e.g., trigger_internal at line 2142, all batch functions, all stream transformers). This means when a user calls task.triggerAndWait(payload, { maxComputeSeconds: 300 }), the maxComputeSeconds value is silently ignored and the run has no compute time limit.
(Refers to line 2399)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🔴 createSchemaTask not updated to use maxComputeSeconds
The createSchemaTask function at packages/trigger-sdk/src/v3/shared.ts:369 still registers maxDuration: params.maxDuration, ignoring the new maxComputeSeconds property. The equivalent createTask function was correctly updated at line 238 to params.maxComputeSeconds ?? params.maxDuration. This means any task defined with schemaTask({ maxComputeSeconds: 300, ... }) will silently have no compute time limit, since params.maxDuration would be undefined.
(Refers to line 369)
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| You must set a default maxDuration in your `trigger.config.ts` file, which will apply to all tasks unless overridden: | ||
| <Note> | ||
| `maxDuration` replaces the deprecated `maxComputeSeconds` parameter. Both work the same way, but we recommend using `maxDuration` for clarity. |
There was a problem hiding this comment.
🔴 Documentation note has maxDuration and maxComputeSeconds swapped
The note in the docs says "maxDuration replaces the deprecated maxComputeSeconds parameter" but this is backwards. The entire PR deprecates maxDuration in favor of maxComputeSeconds, so the note should say "maxComputeSeconds replaces the deprecated maxDuration parameter." This will confuse users about which property is the new recommended one.
| `maxDuration` replaces the deprecated `maxComputeSeconds` parameter. Both work the same way, but we recommend using `maxDuration` for clarity. | |
| `maxComputeSeconds` replaces the deprecated `maxDuration` parameter. Both work the same way, but we recommend using `maxComputeSeconds` for clarity. |
Was this helpful? React with 👍 or 👎 to provide feedback.
| maxComputeSeconds?: number; | ||
|
|
||
| /** | ||
| * @deprecated Use `maxComputeSeconds` instead. This property will be removed in a future version. | ||
| * | ||
| * The maximum duration in compute-time seconds that a task run is allowed to run. If the task run exceeds this duration, it will be stopped. | ||
| * | ||
| * Minimum value is 5 seconds | ||
| * | ||
| * Setting this value will affect all tasks in the project. | ||
| */ | ||
| maxDuration?: number; |
There was a problem hiding this comment.
🚩 Config maxDuration changed from required to optional — potential breaking change for existing configs
In packages/core/src/v3/config.ts:177, maxDuration was previously a required field (maxDuration: number). Now both maxComputeSeconds and maxDuration are optional (maxComputeSeconds?: number and maxDuration?: number). This means existing trigger.config.ts files that relied on maxDuration being required will still compile (since the old field still exists as optional), but new projects that don't set either field will have no default compute time limit. This is likely intentional since it was previously documented as a required field and the templates still set maxComputeSeconds: 3600, but it's worth confirming this behavioral change is desired.
Was this helpful? React with 👍 or 👎 to provide feedback.
| maxComputeSeconds: 300, // 300 seconds or 5 minutes | ||
| run: async (payload: any, { ctx }) => { | ||
| console.log(ctx.run.maxDuration); // 300 | ||
| console.log(ctx.run.maxComputeSeconds); // 300 |
There was a problem hiding this comment.
🚩 ctx.run.maxComputeSeconds in docs references a property that doesn't exist in the runtime context schema
At docs/runs/max-duration.mdx:136, the docs show ctx.run.maxComputeSeconds but the actual run context schema in packages/core/src/v3/schemas/common.ts:227 and packages/core/src/v3/schemas/common.ts:390 still uses maxDuration. The runtime context object sent to the task's run function won't have a maxComputeSeconds property — it will have maxDuration. This doc example will not work as written, though it may be intended to be addressed in a follow-up PR that also renames the context field.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Closes #
✅ Checklist
Testing
[Describe the steps you took to test this change]
Changelog
[Short description of what has changed]
Screenshots
[Screenshots]
💯