feat: Add maxDuration to tasks #1377
Conversation
🦋 Changeset detectedLatest commit: 474d6d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request involve enhancements to the Changes
Suggested reviewers
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 (
|
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: 10
🧹 Outside diff range and nitpick comments (46)
packages/database/prisma/migrations/20241002164627_add_max_duration_in_seconds_to_task_run/migration.sql (1)
1-2: LGTM! Consider default value, NOT NULL constraint, and future optimizations.The migration correctly adds the
maxDurationInSecondscolumn to theTaskRuntable, aligning with the PR objective. However, consider the following suggestions:
Consider adding a default value or a NOT NULL constraint to avoid ambiguity between tasks with no max duration set and those set to unlimited.
Be aware that adding a new column to a large table can be a slow operation, depending on the database system and the number of existing rows. Plan the migration execution accordingly.
If you anticipate frequent queries filtering or sorting by
maxDurationInSeconds, consider adding an index on this column in a future migration to optimize query performance.Would you like assistance in modifying the migration to include these suggestions?
packages/database/prisma/migrations/20241002163757_add_max_duration_to_background_worker_task/migration.sql (1)
1-2: LGTM! Consider additional constraints for robustness.The ALTER TABLE statement correctly adds the "maxDurationInSeconds" column to the "BackgroundWorkerTask" table, aligning with the PR objectives. The INTEGER type is appropriate for storing duration in seconds, and making the column nullable allows for tasks without a specified max duration.
Consider the following suggestions to enhance the robustness of the schema:
Add a CHECK constraint to ensure only non-negative values:
ALTER TABLE "BackgroundWorkerTask" ADD CONSTRAINT "check_max_duration_positive" CHECK ("maxDurationInSeconds" IS NULL OR "maxDurationInSeconds" >= 0);If querying tasks by max duration is expected to be frequent, consider adding an index:
CREATE INDEX "idx_background_worker_task_max_duration" ON "BackgroundWorkerTask" ("maxDurationInSeconds");Decide if NULL is the best representation for "no max duration" or if a large integer value (e.g., 2147483647 for 32-bit INT) would be more appropriate. If you choose the latter, you could set a default:
ALTER TABLE "BackgroundWorkerTask" ALTER COLUMN "maxDurationInSeconds" SET DEFAULT 2147483647;These suggestions are optional and depend on your specific use case and performance requirements. Let me know if you'd like to implement any of these changes.
packages/core/src/v3/timeout-api.ts (1)
1-5: LGTM! Consider adding a brief JSDoc comment.The implementation looks good and aligns with the PR objectives. The use of a singleton pattern for
TimeoutAPIand the consideration for tree-shaking are excellent practices.Consider adding a brief JSDoc comment for the
timeoutexport to improve documentation. For example:import { TimeoutAPI } from "./timeout/api.js"; + +/** Singleton instance of TimeoutAPI for managing task durations */ export const timeout = TimeoutAPI.getInstance();This addition would provide more context for developers using this API.
packages/core/src/v3/timeout/types.ts (2)
1-4: Enhance documentation with JSDoc comments.The
TimeoutManagerinterface is well-structured, but adding JSDoc comments would improve its clarity and usability.Consider adding JSDoc comments as follows:
/** * Manages timeouts for task execution. */ export interface TimeoutManager { /** * Creates an AbortSignal that will be triggered after the specified timeout. * @param timeoutInSeconds The duration in seconds after which the signal should abort. * @returns An AbortSignal that will be triggered after the specified timeout. */ abortAfterTimeout: (timeoutInSeconds: number) => AbortSignal; /** * An optional AbortSignal that can be used to abort the entire operation. */ signal?: AbortSignal; }
6-14: Enhance error message with additional context.The
TaskRunExceededMaxDurationclass is well-implemented, but the error message could be more informative.Consider modifying the error message to include both the timeout and the actual usage duration:
export class TaskRunExceededMaxDuration extends Error { constructor( public readonly timeoutInSeconds: number, public readonly usageInSeconds: number ) { super(`Run exceeded maxDuration of ${timeoutInSeconds} seconds (actual: ${usageInSeconds} seconds)`); this.name = "TaskRunExceededMaxDuration"; } }This change will provide more context about the extent of the duration overrun, which could be helpful for debugging and monitoring.
references/v3-catalog/src/trigger/maxDuration.ts (1)
4-12: LGTM: Well-implemented task withmaxDuration. Consider adding error handling.The
maxDurationTaskcorrectly implements themaxDurationfeature as described in the PR objectives. It properly uses thesignalfor cancellation support and returns useful usage statistics.Consider adding error handling for the
setTimeoutcall. For example:export const maxDurationTask = task({ id: "max-duration", maxDuration: 15, // 15 seconds run: async (payload: { sleepFor: number }, { signal }) => { - await setTimeout(payload.sleepFor * 1000, { signal }); + try { + await setTimeout(payload.sleepFor * 1000, { signal }); + } catch (error) { + logger.error("Task interrupted", { error }); + throw error; + } return usage.getCurrent(); }, });This will provide more context if the task is interrupted due to exceeding the
maxDuration.packages/core/src/v3/schemas/resources.ts (1)
14-14: LGTM! Consider adding validation and documentation.The addition of the
maxDurationproperty to theTaskResourceschema is correct and aligns well with the PR objectives. It's properly typed as an optional number, which is consistent with other optional properties in the schema.To further improve this implementation, consider:
- Adding validation to ensure
maxDurationis a positive number.- Including a comment to explain the purpose and unit of
maxDuration(e.g., seconds, milliseconds).Here's a suggested implementation with these improvements:
/** * Maximum duration allowed for the task execution in milliseconds. * If the task exceeds this duration, it will be aborted. */ maxDuration: z.number().positive().optional(),packages/core/src/v3/utils/globals.ts (1)
60-60: LGTM: Addition of timeout property to TriggerDotDevGlobalAPIThe new
["timeout"]?: TimeoutManager;property is correctly added to theTriggerDotDevGlobalAPItype. This change aligns with the PR objectives, allowing for global registration of a timeout management instance.For consistency with other multi-word properties in this type, consider using kebab-case:
- ["timeout"]?: TimeoutManager; + ["timeout-manager"]?: TimeoutManager;This suggestion is optional and based on the naming convention used for "task-catalog" and "api-client" in the same type.
apps/webapp/app/database-types.ts (1)
45-45: LGTM! Consider alphabetical ordering.The addition of
TIMED_OUTstatus toTaskRunStatusaligns with the PR objectives and maintains type safety. Well done!For better readability and easier maintenance, consider ordering the statuses alphabetically. This would place
TIMED_OUTbeforeWAITING_FOR_DEPLOY.apps/webapp/app/v3/requeueTaskRun.server.ts (1)
73-73: LGTM! Consider adding specific logging for timed-out tasks.The addition of the
TIMED_OUTcase is appropriate and aligns with the PR objectives. The handling is consistent with other completed task states.Consider adding a specific log message for timed-out tasks to aid in monitoring and debugging. For example:
case "TIMED_OUT": case "CANCELED": { - logger.debug("[RequeueTaskRunService] Task run is completed", { taskRun }); + logger.debug("[RequeueTaskRunService] Task run is completed", { taskRun, timedOut: taskRun.status === "TIMED_OUT" }); await marqs?.acknowledgeMessage(taskRun.id); break; }references/v3-catalog/trigger.config.ts (2)
20-20: Add a clarifying comment formaxDuration.The addition of
maxDurationaligns with the PR objectives. However, it would be helpful to add a comment explaining that this value is in seconds and applies globally to all tasks by default.Consider adding a comment like this:
+ // Global default maximum duration for all tasks (in seconds) maxDuration: 60,
37-41: Improved error logging inonFailurecallback.The enhancement to the
onFailurecallback provides more detailed error logging, which is a good improvement for debugging purposes. The implementation correctly distinguishes betweenErrorinstances and other types of errors.Consider updating the PR description to mention this improvement in error logging, as it wasn't included in the original PR objectives.
packages/core/src/v3/types/index.ts (5)
13-14: LGTM! Consider a minor improvement to the comment.The addition of the
signalproperty toRunFnParamsis well-implemented and aligns with the PR objectives. The comment clearly explains its purpose.Consider updating the comment to mention the
maxDurationfeature explicitly:- /** Abort signal that is aborted when a task run exceeds it's maxDuration. Can be used to automatically cancel downstream requests */ + /** Abort signal that is triggered when a task run exceeds its maxDuration. Can be used to automatically cancel downstream requests */
20-21: LGTM! Consider the same minor improvement to the comment.The addition of the
signalproperty toMiddlewareFnParamsis well-implemented and consistent with the changes made toRunFnParams.Consider updating the comment to mention the
maxDurationfeature explicitly:- /** Abort signal that is aborted when a task run exceeds it's maxDuration. Can be used to automatically cancel downstream requests */ + /** Abort signal that is triggered when a task run exceeds its maxDuration. Can be used to automatically cancel downstream requests */
26-27: LGTM! Consider the same minor improvement to the comment.The addition of the
signalproperty toInitFnParamsis well-implemented and consistent with the changes made to other type definitions.Consider updating the comment to mention the
maxDurationfeature explicitly:- /** Abort signal that is aborted when a task run exceeds it's maxDuration. Can be used to automatically cancel downstream requests */ + /** Abort signal that is triggered when a task run exceeds its maxDuration. Can be used to automatically cancel downstream requests */
32-33: LGTM! Consider the same minor improvement to the comment.The addition of the
signalproperty toStartFnParamsis well-implemented and consistent with the changes made to other type definitions.Consider updating the comment to mention the
maxDurationfeature explicitly:- /** Abort signal that is aborted when a task run exceeds it's maxDuration. Can be used to automatically cancel downstream requests */ + /** Abort signal that is triggered when a task run exceeds its maxDuration. Can be used to automatically cancel downstream requests */
68-69: LGTM! Consider the same minor improvement to the comment.The addition of the
signalproperty toHandleErrorArgsis well-implemented and consistent with the changes made to other type definitions.Consider updating the comment to mention the
maxDurationfeature explicitly:- /** Abort signal that is aborted when a task run exceeds it's maxDuration. Can be used to automatically cancel downstream requests */ + /** Abort signal that is triggered when a task run exceeds its maxDuration. Can be used to automatically cancel downstream requests */packages/cli-v3/src/entryPoints/dev-index-worker.ts (2)
102-114: LGTM: Correct implementation of globalmaxDurationThis block correctly implements the global
maxDurationfeature as per the PR objectives:
- It only applies the global setting to tasks without a specific
maxDuration.- It uses immutable update patterns, which is good for data integrity.
Consider a minor optimization: you can avoid reassigning
tasksifconfig.maxDurationis not a number. Here's a suggested refactor:- let tasks = taskCatalog.listTaskManifests(); + const tasks = taskCatalog.listTaskManifests(); - // If the config has a maxDuration, we need to apply it to all tasks that don't have a maxDuration - if (typeof config.maxDuration === "number") { - tasks = tasks.map((task) => { + // Apply global maxDuration to tasks without a specific maxDuration + const updatedTasks = typeof config.maxDuration === "number" + ? tasks.map((task) => { if (typeof task.maxDuration !== "number") { return { ...task, maxDuration: config.maxDuration, }; } return task; - }); - } + }) + : tasks;This approach maintains the original
tasksconstant and creates a newupdatedTasksvariable only if necessary.
Line range hint
1-165: Overall implementation looks good, with some considerationsThe changes in this file successfully implement the global
maxDurationfeature as described in the PR objectives. The implementation correctly prioritizes task-specificmaxDurationover the global setting and uses good practices like immutable updates.However, there are a few points to consider:
The PR objectives mention that
maxDurationcan be set in three contexts (global, task-specific, and on-the-fly override), but this file only handles the global context. Ensure that the other contexts are handled in appropriate parts of the codebase.There's no explicit error handling or validation for the
maxDurationvalue. Consider adding validation to ensure thatmaxDurationis a positive number.Consider the following improvements:
Add validation for the
maxDurationvalue:if (typeof config.maxDuration === "number" && config.maxDuration > 0) { // Apply maxDuration } else if (config.maxDuration !== undefined) { console.warn(`Invalid maxDuration: ${config.maxDuration}. It should be a positive number.`); }If not already implemented elsewhere, ensure that the task-specific and on-the-fly override of
maxDurationare handled in the appropriate parts of the codebase.Consider adding documentation or comments explaining the
maxDurationfeature and how it's applied at different levels (global, task-specific, on-the-fly).Would you like assistance in implementing any of these suggestions or in reviewing other parts of the codebase where the
maxDurationfeature is implemented?packages/core/src/v3/config.ts (1)
40-47: Enhance documentation formaxDurationpropertyThe addition of the
maxDurationproperty is well-implemented and aligns with the PR objectives. However, the documentation could be improved for clarity and completeness:
- Specify the default behavior when
maxDurationis not set.- Consider adding a maximum allowed value for
maxDuration.- Clarify the term "compute-time seconds" - does this exclude time spent in I/O operations?
- Add an example usage to illustrate how to set this property.
Here's a suggested improvement for the documentation:
/** * The maximum duration in compute-time seconds that a task run is allowed to execute. If the task run exceeds this duration, it will be stopped. * * @remarks * - Compute-time seconds refer to the actual CPU time used by the task, excluding time spent in I/O operations or `wait.for` calls. * - This setting affects all tasks in the project, but can be overridden on a per-task basis. * - If not set, tasks can run indefinitely (subject to any global system limits). * * @minimum 5 * @maximum [Please specify a maximum value if applicable] * * @example * ``` * export default new Trigger({ * id: "example", * name: "Example Trigger", * maxDuration: 300, // Set maximum duration to 5 minutes * // ... other configuration * }); * ``` */ maxDuration?: number;Could you please clarify if there's a maximum allowed value for
maxDuration? Also, confirm if the interpretation of "compute-time seconds" is correct.apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (1)
69-69: Approve with minor suggestion: TIMED_OUT description added.The description for the new "TIMED_OUT" status has been added correctly. However, there's a minor grammatical error in the description.
Consider changing "it's" to "its" for grammatical correctness:
- TIMED_OUT: "Task has reached it's maxDuration and has been stopped", + TIMED_OUT: "Task has reached its maxDuration and has been stopped",packages/core/src/v3/schemas/schemas.ts (2)
157-157: LGTM! Consider adding a brief comment for clarity.The addition of the
maxDurationfield to thetaskMetadataobject is correct and aligns well with the PR objectives. It's properly defined as an optional number using Zod, maintaining backward compatibility.Consider adding a brief comment to explain the purpose and unit of
maxDuration. For example:/** Maximum duration (in milliseconds) for the task execution. */ maxDuration: z.number().optional(),This would improve code readability and make it clear that the duration is in milliseconds.
Line range hint
211-241: Consider adding a global defaultmaxDurationto theConfigschemaThe PR objectives mention that
maxDurationcan be set as a global default in thetrigger.config.tsfile. To support this, we should add amaxDurationfield to theConfigschema.Consider adding the following field to the
Configschema:maxDuration: z.number().optional(),This would allow users to set a global default
maxDurationfor all tasks in their configuration file.apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (1)
199-199: LGTM! Consider adding a comment for clarity.The addition of the
maxDurationproperty to theexecution.runobject aligns well with the PR objectives. It correctly implements the new feature allowing tasks to have a maximum duration.For improved clarity, consider adding a brief comment explaining the purpose of this property:
+ // Maximum allowed duration for the task in seconds, if set maxDuration: taskRun.maxDurationInSeconds ?? undefined,packages/core/src/v3/schemas/common.ts (2)
149-149: LGTM with a minor suggestion: Addition of maxDuration to TaskRun schemaThe addition of the
maxDurationproperty to theTaskRunschema directly implements the feature described in the PR objectives. It's correctly set as optional and uses an appropriate data type (number).Consider adding a comment or using a more descriptive type (e.g.,
z.number().positive().int()) to clarify the expected unit of measurement (milliseconds, seconds, etc.) for themaxDurationproperty. This would enhance code readability and prevent potential misuse.
Line range hint
95-149: Summary: Successful implementation of maxDuration featureThe changes in this file successfully implement the necessary components for the
maxDurationfeature as described in the PR objectives. The additions include:
- A new error code for maximum duration exceeded.
- Integration of this error code into the internal error handling system.
- A new optional property in the
TaskRunschema to specify the maximum duration.These changes provide the foundation for setting and enforcing maximum task durations at various levels (global, task-specific, and on-the-fly) as outlined in the PR objectives. The implementation is consistent with existing code patterns and structures.
To fully realize the PR objectives, ensure that:
- The
trigger.config.tsfile is updated to allow setting a global defaultmaxDuration.- The task definition interface is updated to include the
maxDurationproperty.- The
triggercommand implementation is modified to accept and overridemaxDuration.- The task execution logic is updated to enforce the
maxDurationlimit and generate the appropriate error when exceeded.apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1)
313-315: LGTM! Consider adding a comment for consistency.The addition of the "TIMED_OUT" case is correct and aligns with the PR objectives. It properly handles the new status introduced by the
maxDurationfeature.For consistency with other cases, consider adding a comment explaining the TIMED_OUT status:
case "TIMED_OUT": { + // Task exceeded its maximum allowed duration return "TIMED_OUT"; }apps/webapp/app/presenters/v3/SpanPresenter.server.ts (1)
233-233: LGTM: Addition ofmaxDurationto context objectThe addition of the
maxDurationproperty to therunobject within the context is correct and aligns with the PR objective. The use of the nullish coalescing operator is appropriate for handling potential undefined values.For improved clarity, consider using an explicit type annotation:
-maxDuration: run.maxDurationInSeconds ?? undefined, +maxDuration: run.maxDurationInSeconds as number | undefined,This change would make the intended type more explicit, potentially improving code readability and type safety.
apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1)
273-275: LGTM! Consider a minor formatting adjustment.The addition of the "TIMED_OUT" case is correct and aligns with the PR objective of introducing a
maxDurationfeature. This change allows the API to properly represent runs that have exceeded their maximum duration.For consistency with the other cases, consider removing the curly braces:
- case "TIMED_OUT": { - return "TIMED_OUT"; - } + case "TIMED_OUT": + return "TIMED_OUT";packages/core/src/v3/errors.ts (1)
61-65: Improved error handling for INTERNAL_ERROR cases.The changes enhance the specificity of internal error handling, which is a good improvement. The new implementation preserves more information about the error, including the error code and stack trace.
Consider adding a comment explaining the purpose of this change, especially if it's related to the
maxDurationfeature mentioned in the PR objectives. This would help maintain context for future developers.case "INTERNAL_ERROR": { + // Enhance internal error information for better debugging, especially for maxDuration-related errors const e = new Error(error.message ?? `Internal error (${error.code})`); e.name = error.code; e.stack = error.stackTrace; return e; }packages/cli-v3/src/dev/workerRuntime.ts (1)
313-314: LGTM! Consider adding more context to the debug log.The addition of this debug logging statement is a good improvement for observability. It will help in troubleshooting and monitoring task executions.
To make the log even more useful, consider adding more context such as the
idandpayload.runId. This would help correlate the log with specific task runs:- logger.debug("Executing task run lazy attempt", { execution }); + logger.debug("Executing task run lazy attempt", { id, runId: payload.runId, execution });apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1)
159-159: LGTM! Consider adding a comment for clarity.The addition of
maxDurationInSecondsaligns well with the PR objective of introducing amaxDurationfeature for tasks. The implementation correctly handles both the presence and absence oftask.maxDuration, and the minimum value of 5 seconds is a reasonable lower bound.Consider adding a brief comment explaining the purpose of the
Math.maxoperation and the minimum value of 5 seconds for clarity:- maxDurationInSeconds: task.maxDuration ? Math.max(task.maxDuration, 5) : null, + // Ensure maxDuration is at least 5 seconds to prevent extremely short durations + maxDurationInSeconds: task.maxDuration ? Math.max(task.maxDuration, 5) : null,apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (2)
381-382: LGTM! Consider adding a comment for clarity.The implementation of
maxDurationInSecondsaligns well with the PR objectives. It correctly prioritizes the task-specific duration over the background task's default value, ensuring proper assignment of the maximum duration for task execution.Consider adding a brief comment explaining the precedence of
maxDurationInSecondsvalues for improved code readability:maxDurationInSeconds: + // Prioritize task-specific duration over background task's default existingTaskRun.maxDurationInSeconds ?? backgroundTask.maxDurationInSeconds,
Line range hint
374-383: Consider destructuring for consistency and future-proofing.The implementation of
maxDurationInSecondsis well-integrated into the existing logic. To improve consistency and make future additions easier, consider destructuring the properties fromexistingTaskRunandbackgroundTask.Here's a suggested refactoring:
+ const { maxDurationInSeconds: existingMaxDuration, startedAt } = existingTaskRun; + const { maxDurationInSeconds: defaultMaxDuration } = backgroundTask; const lockedTaskRun = await prisma.taskRun.update({ where: { id: message.messageId, }, data: { lockedAt: new Date(), lockedById: backgroundTask.id, status: "EXECUTING", lockedToVersionId: backgroundWorker.id, - startedAt: existingTaskRun.startedAt ?? new Date(), + startedAt: startedAt ?? new Date(), maxDurationInSeconds: - existingTaskRun.maxDurationInSeconds ?? backgroundTask.maxDurationInSeconds, + existingMaxDuration ?? defaultMaxDuration, }, // ... rest of the update operation });This approach makes the code more readable and easier to maintain if additional properties need to be added in the future.
packages/core/src/v3/schemas/api.ts (1)
87-87: LGTM! Consider adding a comment for clarity.The addition of the
maxDurationfield aligns well with the PR objectives. It's correctly defined as an optional number, which is appropriate for representing duration.Consider adding a brief comment to explain the purpose and unit of measurement for
maxDuration. For example:/** Maximum duration (in seconds) for task execution. If exceeded, the run will be aborted. */ maxDuration: z.number().optional(),apps/webapp/app/components/runs/v3/RunInspector.tsx (2)
327-332: LGTM! Consider adding a tooltip for clarity.The implementation of the "Max duration" property item is correct and aligns well with the PR objectives. It properly displays the maximum duration in seconds when available or shows a placeholder when not set.
To enhance user understanding, consider adding a tooltip that explains what the max duration represents and how it affects the run. For example:
<Property.Item> <Property.Label>Max duration</Property.Label> <Property.Value> - {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <SimpleTooltip + button={ + <span>{run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"}</span> + } + content="Maximum allowed CPU time for task execution. The run will be aborted if exceeded." + /> </Property.Value> </Property.Item>This addition would provide users with immediate context about the purpose and impact of the max duration setting.
Line range hint
1-732: Overall implementation looks good. Consider updating component documentation.The addition of the "Max duration" property to the
RunInspectorcomponent has been implemented correctly and aligns well with the PR objectives. The changes are consistent with the existing code style and structure.To maintain comprehensive documentation, consider updating the component's JSDoc or comments to reflect the new "Max duration" property. This will help future developers understand the full range of information displayed by the
RunInspector.Example:
/** * The RunInspector displays live information about a run. * ... * @property {number} run.maxDurationInSeconds - The maximum allowed CPU time for task execution in seconds. * ... */ export function RunInspector({ // ... })apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam.spans.$spanParam/route.tsx (2)
680-685: Consider adding a tooltip for "Max duration".The "Max duration" property has been added successfully. To enhance user understanding, consider adding a tooltip that explains what this property represents and how it affects the run.
You could implement this using the
InfoIconTooltipcomponent that's used elsewhere in the file. Here's a suggested implementation:<Property.Item> <Property.Label>Max duration</Property.Label> <Property.Value> - {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <span className="flex items-center gap-1"> + {run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"} + <InfoIconTooltip + content="Maximum allowed duration for this run. The run will be aborted if it exceeds this time." + contentClassName="normal-case tracking-normal" + /> + </span> </Property.Value> </Property.Item>This change would provide users with more context about the "Max duration" feature directly in the UI.
Ensure "Max duration" is integrated into all instances of
RunOverview.The addition of the "Max duration" property in the
RunBodycomponent is a positive enhancement. However, since theRunOverviewcomponent is utilized in multiple parts of the codebase, please ensure that the "Max duration" is also incorporated withinRunOverview. This will maintain consistency and ensure that the "Max duration" is displayed whereverRunOverviewis used.Consider updating the following files to include the "Max duration" property in the
RunOverviewcomponent:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.triggers_.webhooks.$triggerParam_.runs.delivery.$runParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.triggers_.external.$triggerParam_.runs.$runParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.jobs.$jobParam.runs.$runParam/route.tsxapps/webapp/app/components/run/RunOverview.tsx🔗 Analysis chain
Line range hint
1-1010: Ensure proper integration of the "Max duration" feature.The addition of the "Max duration" property is well-implemented within the
RunBodycomponent. However, given the size and complexity of this file, it's important to ensure that this new feature is fully integrated across all relevant parts of the component.Consider the following:
- If there are any other places in this file where run properties are displayed or used, make sure the "Max duration" is included where appropriate.
- Verify that any functions or components that process or display run data are updated to handle the
maxDurationInSecondsproperty.- If there are any summary or overview sections of the run information, consider including the max duration there as well.
To help ensure proper integration, you can run the following script to check for other potential places where max duration might need to be included:
This will help identify any other areas in the codebase where the max duration might need to be incorporated for consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other places where run properties are displayed or used # Search for patterns related to displaying run properties rg --type tsx 'run\.(duration|time|seconds)' # Search for components or functions that might need updating rg --type tsx '(RunSummary|RunOverview|RunDetails)'Length of output: 301
Script:
#!/bin/bash # Description: Check for other places where run properties are displayed or used # Search for patterns related to displaying run properties in .tsx files rg 'run\.(duration|time|seconds)' --glob "*.tsx" # Search for components or functions that might need updating in .tsx files rg '(RunSummary|RunOverview|RunDetails)' --glob "*.tsx"Length of output: 1677
packages/database/prisma/schema.prisma (2)
1637-1638: LGTM! Consider adding a comment for clarity.The addition of the
maxDurationInSecondsfield to theBackgroundWorkerTaskmodel is appropriate and aligns with the PR objectives. It allows for setting a maximum duration for task execution.Consider adding a brief comment above the field to explain its purpose and any default behavior, e.g.:
/// Maximum duration in seconds for task execution. If null, no time limit is applied. maxDurationInSeconds Int?
1774-1775: LGTM! Consider adding a comment for consistency.The addition of the
maxDurationInSecondsfield to theTaskRunmodel is appropriate and consistent with theBackgroundWorkerTaskmodel. It allows for setting a maximum duration for individual task runs.For consistency with the suggested improvement in the
BackgroundWorkerTaskmodel, consider adding a brief comment above the field:/// Maximum duration in seconds for this task run. If null, no time limit is applied. maxDurationInSeconds Int?packages/core/src/v3/timeout/usageTimeoutManager.ts (1)
4-14: Consider making the_abortSignalpropertyreadonly.Since the
_abortSignalproperty is only assigned a value in the constructor and theabortAfterTimeoutmethod, and is not intended to be reassigned elsewhere, consider marking it asreadonlyto ensure immutability and prevent accidental reassignment.-private _abortSignal: AbortSignal | undefined; +private readonly _abortSignal: AbortSignal | undefined;packages/core/src/v3/timeout/api.ts (2)
4-4: Consider using a more descriptive constant name.The constant name
API_NAMEis a bit generic. Consider using a more specific name that clearly indicates the purpose of this constant, such asTIMEOUT_API_NAMEorTIMEOUT_MANAGER_KEY.-const API_NAME = "timeout"; +const TIMEOUT_API_NAME = "timeout";
19-25: Consider using the class name instead ofthisin a static context.Using
thisin a static context can be confusing because it refers to the class itself rather than an instance of the class. To improve clarity, consider using the class nameTimeoutAPIinstead ofthis.public static getInstance(): TimeoutAPI { - if (!this._instance) { - this._instance = new TimeoutAPI(); + if (!TimeoutAPI._instance) { + TimeoutAPI._instance = new TimeoutAPI(); } - return this._instance; + return TimeoutAPI._instance; }🧰 Tools
🪛 Biome
[error] 20-20: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 24-24: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
packages/cli-v3/src/entryPoints/dev-run-worker.ts (1)
Line range hint
1-386: Consider adding tests for the new timeout functionality.The changes introduce a significant new feature for managing task execution timeouts. To ensure the robustness and reliability of this feature, consider adding comprehensive unit tests that cover various scenarios, such as:
- Task completing successfully within the specified timeout.
- Task exceeding the specified timeout and being aborted.
- Task not having a specified timeout and running to completion.
Do you want me to provide guidance on how to structure these tests or assist in writing them?
apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (1)
Line range hint
1-1145: Consider adding tests for themaxDurationfunctionality.While the code changes related to
maxDurationlook good, it's important to ensure that this new functionality is thoroughly tested. Consider adding unit tests that cover scenarios such as:
- Task runs with
maxDurationset execute and terminate correctly when the duration is exceeded.- Task runs without
maxDurationset execute normally and don't have any duration limit enforced.- The priority order of
maxDurationsettings (global, task-specific, on-the-fly) is correctly applied.Adding tests will help catch any potential issues and ensure the reliability of the
maxDurationfeature.Do you want me to generate some example test cases for the
maxDurationfunctionality?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (46)
- apps/webapp/app/components/runs/v3/RunInspector.tsx (1 hunks)
- apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (5 hunks)
- apps/webapp/app/database-types.ts (1 hunks)
- apps/webapp/app/models/taskRun.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts (3 hunks)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam.spans.$spanParam/route.tsx (1 hunks)
- apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (1 hunks)
- apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (2 hunks)
- apps/webapp/app/v3/requeueTaskRun.server.ts (1 hunks)
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts (1 hunks)
- apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (1 hunks)
- apps/webapp/app/v3/services/triggerTask.server.ts (1 hunks)
- packages/cli-v3/src/commands/init.ts (0 hunks)
- packages/cli-v3/src/dev/workerRuntime.ts (1 hunks)
- packages/cli-v3/src/entryPoints/deploy-index-worker.ts (1 hunks)
- packages/cli-v3/src/entryPoints/deploy-run-worker.ts (4 hunks)
- packages/cli-v3/src/entryPoints/dev-index-worker.ts (1 hunks)
- packages/cli-v3/src/entryPoints/dev-run-worker.ts (3 hunks)
- packages/core/src/v3/config.ts (1 hunks)
- packages/core/src/v3/errors.ts (1 hunks)
- packages/core/src/v3/index.ts (1 hunks)
- packages/core/src/v3/schemas/api.ts (2 hunks)
- packages/core/src/v3/schemas/common.ts (3 hunks)
- packages/core/src/v3/schemas/resources.ts (1 hunks)
- packages/core/src/v3/schemas/schemas.ts (1 hunks)
- packages/core/src/v3/timeout-api.ts (1 hunks)
- packages/core/src/v3/timeout/api.ts (1 hunks)
- packages/core/src/v3/timeout/types.ts (1 hunks)
- packages/core/src/v3/timeout/usageTimeoutManager.ts (1 hunks)
- packages/core/src/v3/tracer.ts (4 hunks)
- packages/core/src/v3/types/index.ts (2 hunks)
- packages/core/src/v3/utils/globals.ts (2 hunks)
- packages/core/src/v3/workers/index.ts (1 hunks)
- packages/core/src/v3/workers/taskExecutor.ts (22 hunks)
- packages/database/prisma/migrations/20241002155751_add_timed_out_status_to_task_run_status/migration.sql (1 hunks)
- packages/database/prisma/migrations/20241002163757_add_max_duration_to_background_worker_task/migration.sql (1 hunks)
- packages/database/prisma/migrations/20241002164627_add_max_duration_in_seconds_to_task_run/migration.sql (1 hunks)
- packages/database/prisma/schema.prisma (3 hunks)
- packages/trigger-sdk/src/v3/shared.ts (7 hunks)
- references/hello-world/src/trigger/example.ts (2 hunks)
- references/hello-world/trigger.config.ts (1 hunks)
- references/v3-catalog/package.json (1 hunks)
- references/v3-catalog/src/trigger/maxDuration.ts (1 hunks)
- references/v3-catalog/trigger.config.ts (2 hunks)
💤 Files with no reviewable changes (1)
- packages/cli-v3/src/commands/init.ts
🧰 Additional context used
🪛 Biome
packages/core/src/v3/timeout/api.ts
[error] 20-20: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 24-24: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
🔇 Additional comments (115)
references/hello-world/trigger.config.ts (2)
Line range hint
1-17: Consider the broader impact of introducingmaxDurationThe introduction of
maxDurationin the global configuration is a significant change that could affect all tasks in the project. To ensure a smooth integration of this feature:
- Verify that all existing tasks are compatible with this new global timeout.
- Update project documentation to explain the
maxDurationfeature, including how to set it globally, per-task, and during task invocation.- Consider adding a migration guide or changelog entry to inform users about this new feature and its potential impact on existing tasks.
- Ensure that error handling and logging mechanisms are in place to clearly communicate when a task has been terminated due to exceeding
maxDuration.To assess the potential impact, let's search for task definitions in the project:
#!/bin/bash # Search for task definitions echo "Searching for task definitions:" rg --type typescript "defineTask|createTask" -C 3 echo "\nSearching for existing timeout or duration-related code:" rg --type typescript "timeout|duration" -C 3This will help identify tasks that might be affected by the new global
maxDurationand any existing timeout-related code that might interact with this new feature.
6-6: Verify themaxDurationimplementation and consider adding documentation.The addition of
maxDuration: 60to the global configuration aligns with the PR objectives of introducing a global default for task duration. However, there are a few points to consider:
- The value 60 is likely in seconds, but this isn't explicitly stated. Consider adding a comment or using a more explicit value like
60 * 1000if it's meant to be in milliseconds.- It might be beneficial to add a comment explaining the purpose of this setting, especially mentioning that it can be overridden at the task level or during task invocation.
Consider updating the line as follows:
maxDuration: 60, // Global default max duration for tasks in secondsTo ensure this change is properly documented and used consistently, let's check for any related changes or documentation:
✅ Verification successful
Confirm
maxDurationis consistently in seconds and enhance clarity with comments.The
maxDuration: 60added to the global configuration aligns with its usage across the codebase, wheremaxDurationis specified in seconds. To improve clarity and maintain consistency:
- Add a comment to specify that
maxDurationis measured in seconds.maxDuration: 60, // Global default max duration for tasks in secondsThis ensures that future developers understand the unit of measurement, reducing potential confusion.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for maxDuration usage and documentation echo "Searching for maxDuration usage:" rg --type typescript "maxDuration" -C 3 echo "\nSearching for maxDuration documentation:" rg --type markdown "maxDuration"Length of output: 368
Script:
#!/bin/bash # Retry searching for maxDuration usage and documentation without specifying file type echo "Searching for maxDuration usage across all files:" rg "maxDuration" -C 3 echo "\nSearching for maxDuration documentation across all files:" rg "maxDuration"Length of output: 35850
packages/core/src/v3/timeout/types.ts (1)
1-14: Overall, the changes look good and align with the PR objectives.The introduced
TimeoutManagerinterface andTaskRunExceededMaxDurationclass provide a solid foundation for implementing themaxDurationfeature for tasks. They effectively support the ability to set maximum durations for task execution and handle cases where these durations are exceeded.The types are well-structured and should integrate smoothly with the rest of the system. With the suggested minor improvements to documentation and error messaging, this implementation will greatly enhance the task management capabilities of the project.
references/v3-catalog/src/trigger/maxDuration.ts (2)
1-2: LGTM: Appropriate imports for the defined tasks.The import statements are correct and import the necessary modules for the tasks defined in this file. The use of
setTimeoutfrom "timers/promises" is a good choice for promise-based timer functionality.
1-21: Summary: Good implementation ofmaxDurationfeature with minor improvements needed.Overall, the implementation of the
maxDurationfeature aligns well with the PR objectives. ThemaxDurationTaskcorrectly implements the feature, while themaxDurationParentTaskdemonstrates how to use it in a parent-child task relationship.Key points:
- Consider adding error handling in
maxDurationTask.- Review and possibly adjust the
maxDurationvalue inmaxDurationParentTask.- Clean up unused parameters in
maxDurationParentTask.These improvements will enhance the robustness and clarity of the implementation.
packages/core/src/v3/workers/index.ts (1)
16-16: LGTM: New export aligns with PR objectivesThe addition of the
UsageTimeoutManagerexport is consistent with the PR's objective of introducing amaxDurationfeature for tasks. This export will allow other parts of the codebase to utilize the timeout management functionality, which is crucial for implementing the task duration limits.A few points to consider:
- The export statement follows the existing pattern in the file.
- The naming convention (
UsageTimeoutManager) is consistent with other exported entities.- The relative import path (
../timeout/usageTimeoutManager.js) suggests proper module organization.To ensure this change doesn't introduce any issues, let's verify the existence and structure of the
UsageTimeoutManager:This script will help verify that the
UsageTimeoutManageris properly implemented and contains methods relevant to themaxDurationfeature.packages/core/src/v3/schemas/resources.ts (1)
Line range hint
1-41: Verify the integration ofmaxDurationacross the codebaseThe addition of
maxDurationto theTaskResourceschema is correct. However, to ensure full implementation of the feature as described in the PR objectives, we should verify its integration in other parts of the codebase.Please run the following script to check for the implementation of
maxDurationin other relevant files:This script will help us confirm that
maxDurationis properly implemented in:
- The global configuration file
- Task definition files
- Task execution logic
- CLI command files for on-the-fly override
✅ Verification successful
Verify the integration of
maxDurationacross the codebaseThe addition of
maxDurationto theTaskResourceschema is correctly implemented and has been thoroughly integrated across relevant parts of the codebase, including configuration files, task definitions, execution logic, and CLI commands.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for maxDuration implementation in config and task execution files # Test 1: Check for maxDuration in trigger.config.ts (for global default) echo "Checking trigger.config.ts for maxDuration:" rg --type typescript 'maxDuration' $(fd -t f 'trigger.config.ts') # Test 2: Check for maxDuration in task definition files echo "\nChecking task definition files for maxDuration:" rg --type typescript 'maxDuration' $(fd -t f '.*task.*\.ts') # Test 3: Check for maxDuration in task execution logic echo "\nChecking task execution logic for maxDuration:" rg --type typescript 'maxDuration' $(fd -t f '(run|execute).*task.*\.ts') # Test 4: Check for maxDuration in CLI command files (for on-the-fly override) echo "\nChecking CLI command files for maxDuration:" rg --type typescript 'maxDuration' $(fd -t f 'trigger.*command.*\.ts')Length of output: 6056
Script:
#!/bin/bash # Description: Check for maxDuration implementation in config and task execution files # Test 1: Check for maxDuration in trigger.config.ts (for global default) echo "Checking trigger.config.ts for maxDuration:" rg 'maxDuration' $(fd -t f 'trigger.config.ts') # Test 2: Check for maxDuration in task definition files echo -e "\nChecking task definition files for maxDuration:" rg 'maxDuration' $(fd -t f -e ts -e tsx '.*task.*\.ts') # Test 3: Check for maxDuration in task execution logic echo -e "\nChecking task execution logic for maxDuration:" rg 'maxDuration' $(fd -t f -e ts -e tsx '(run|execute).*task.*\.ts') # Test 4: Check for maxDuration in CLI command files (for on-the-fly override) echo -e "\nChecking CLI command files for maxDuration:" rg 'maxDuration' $(fd -t f -e ts -e tsx 'trigger.*command.*\.ts')Length of output: 13557
packages/core/src/v3/utils/globals.ts (2)
7-7: LGTM: New import for TimeoutManagerThe import statement for
TimeoutManageris correctly added and follows the existing import style in the file. This import is necessary for the subsequent addition to theTriggerDotDevGlobalAPItype.
Line range hint
1-61: Verify complete implementation of maxDuration featureThe changes in this file successfully introduce the groundwork for the
maxDurationfeature by adding timeout management capabilities to the global API. These modifications are minimal, focused, and backwards-compatible.However, the actual implementation of the
maxDurationfeature, as described in the PR objectives (global default intrigger.config.ts, task-specific settings, and on-the-fly overrides), is not present in this file.Please ensure that the complete implementation of the
maxDurationfeature is present in other files within this PR. You can use the following script to check for relevant changes:✅ Verification successful
Verified:
maxDurationfeature is fully implementedThe
maxDurationfeature has been successfully implemented across the codebase. All necessary changes related to timeout management and themaxDurationsettings are present and correctly integrated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for maxDuration implementation in other files # Search for maxDuration in TypeScript files echo "Searching for maxDuration implementation:" rg --type typescript 'maxDuration' -g '!packages/core/src/v3/utils/globals.ts' # Search for changes in trigger.config.ts echo "\nChecking for changes in trigger.config.ts:" git diff origin/main -- '**/trigger.config.ts' # Search for changes in task definition files echo "\nChecking for changes in task definition files:" git diff origin/main -- '**/task*.ts' # Search for changes in trigger command files echo "\nChecking for changes in trigger command files:" git diff origin/main -- '**/trigger*.ts'Length of output: 19347
Script:
#!/bin/bash # Description: Check for maxDuration implementation in other files # Search for maxDuration in TypeScript files echo "Searching for maxDuration implementation:" rg --type ts 'maxDuration' -g '!packages/core/src/v3/utils/globals.ts' # Search for changes in trigger.config.ts echo "\nChecking for changes in trigger.config.ts:" git diff origin/main -- '**/trigger.config.ts' # Search for changes in task definition files echo "\nChecking for changes in task definition files:" git diff origin/main -- '**/task*.ts' # Search for changes in trigger command files echo "\nChecking for changes in trigger command files:" git diff origin/main -- '**/trigger*.ts'Length of output: 25788
packages/core/src/v3/index.ts (1)
15-15: LGTM! Verify the implementation ofmaxDurationfeature.The addition of the export for "timeout-api.js" aligns with the PR objectives to introduce the
maxDurationfeature for tasks. The placement maintains the alphabetical order of exports, which is good for readability.To ensure the completeness of the implementation, please run the following script to verify the contents of the "timeout-api.js" file:
This will help confirm that the
maxDurationfeature is properly implemented in the exported file.apps/webapp/app/database-types.ts (2)
Line range hint
58-58: LGTM! Consistent implementation.The addition of
TIMED_OUTstatus toJobRunStatusis consistent with the changes inTaskRunStatusand aligns with the PR objectives. The alphabetical ordering is maintained, which is great for readability.
45-45: Verify handling of newTIMED_OUTstatus across the codebase.The addition of
TIMED_OUTstatus to bothTaskRunStatusandJobRunStatusis consistent and aligns with the PR objectives. However, it's crucial to ensure that other parts of the codebase are updated to handle this new status appropriately.Please run the following script to check for potential places where the new status needs to be handled:
Review the results to ensure that the
TIMED_OUTstatus is properly handled in all relevant places.Also applies to: 58-58
apps/webapp/app/v3/requeueTaskRun.server.ts (1)
Line range hint
1-101: Verify complete implementation ofmaxDurationfeatureThe addition of the
TIMED_OUTcase in theRequeueTaskRunServiceis a good start for implementing themaxDurationfeature. However, to ensure full implementation, we should verify that other parts of the codebase have been updated accordingly.Please run the following script to check for other relevant changes:
This script will help us verify that the
maxDurationfeature has been implemented in the global config, task definitions, and trigger command handling, as mentioned in the PR objectives.references/v3-catalog/trigger.config.ts (1)
Line range hint
1-41: Verify implementation of task-specific and on-the-flymaxDurationsettings.The PR objectives mention that
maxDurationcan be set in three contexts: global (implemented here), task-specific, and on-the-fly. While the global setting is correctly implemented in this file, the other two contexts are not visible.Could you please confirm that the task-specific and on-the-fly
maxDurationsettings are implemented elsewhere in the codebase? If so, it would be helpful to see those implementations to ensure consistency across all three contexts.To verify this, we can run the following script:
packages/core/src/v3/types/index.ts (1)
Line range hint
1-69: Overall implementation of themaxDurationfeature is well-executed.The changes made to this file consistently implement the
maxDurationfeature across all relevant type definitions by adding an optionalsignalproperty of typeAbortSignal. This aligns perfectly with the PR objectives to introduce a mechanism for aborting tasks that exceed their maximum duration.The implementation enhances control over task execution and provides a uniform way to handle task cancellation across different function types (run, middleware, init, start, and error handling).
Great job on maintaining consistency throughout the changes!
apps/webapp/app/models/taskRun.server.ts (2)
122-122: LGTM: Correctly handles the new TIMED_OUT statusThe addition of
TaskRunStatus.TIMED_OUTcase is correct and aligns well with the PR objectives. It ensures that tasks exceeding theirmaxDurationwill be marked as failed in batch task runs, consistent with other failure statuses.
122-122: Verify complete implementation of maxDuration featureWhile this change correctly handles the TIMED_OUT status for batch task runs, it's important to ensure that all necessary changes for the
maxDurationfeature have been implemented across the codebase. This includes the ability to setmaxDurationglobally, per task, and via thetriggercommand as described in the PR objectives.To assist in verifying the complete implementation, you can run the following script:
This script will help identify if the
maxDurationfeature has been fully implemented as described in the PR objectives.✅ Verification successful
Confirmation of Complete
maxDurationFeature ImplementationThe
maxDurationfeature has been thoroughly implemented across the codebase, including global configurations, task-specific settings, and trigger commands. Additionally, the handling of theTIMED_OUTstatus is consistently managed in multiple modules, ensuring robust functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for maxDuration implementation across the codebase # Check for global maxDuration configuration echo "Checking for global maxDuration configuration:" rg --type typescript 'maxDuration.*trigger\.config\.ts' # Check for task-specific maxDuration echo "\nChecking for task-specific maxDuration:" rg --type typescript 'maxDuration.*task' # Check for maxDuration in trigger command echo "\nChecking for maxDuration in trigger command:" rg --type typescript 'maxDuration.*trigger' # Check for TIMED_OUT status handling echo "\nChecking for TIMED_OUT status handling:" rg --type typescript 'TIMED_OUT'Length of output: 711
Script:
#!/bin/bash # Description: Check for maxDuration implementation across the codebase with corrected file type specifications # Check for global maxDuration configuration echo "Checking for global maxDuration configuration:" rg 'maxDuration' --glob '*.ts' --glob '*.tsx' trigger.config.ts # Check for task-specific maxDuration echo "\nChecking for task-specific maxDuration:" rg 'maxDuration' --glob '*.ts' --glob '*.tsx' task # Check for maxDuration in trigger command echo "\nChecking for maxDuration in trigger command:" rg 'maxDuration' --glob '*.ts' --glob '*.tsx' trigger # Check for TIMED_OUT status handling echo "\nChecking for TIMED_OUT status handling:" rg 'TIMED_OUT' --glob '*.ts' --glob '*.tsx'Length of output: 3988
Script:
#!/bin/bash # Description: Comprehensive search for maxDuration implementation across all TypeScript files echo "Searching for 'maxDuration' in all .ts and .tsx files:" rg 'maxDuration' --type-add 'ts:*.ts' --type-add 'tsx:*.tsx' -t ts -t tsxLength of output: 6682
packages/cli-v3/src/entryPoints/dev-index-worker.ts (2)
98-101: LGTM: Necessary changes formaxDurationimplementationThe changes in these lines are crucial for implementing the
maxDurationfeature:
- Extracting the
configobject allows access to the globalmaxDurationsetting.- Changing
taskstoletenables its modification to apply the globalmaxDuration.These modifications align well with the PR objectives.
Line range hint
116-165: Verify usage of modifiedtasksvariableThe remaining part of the file uses the
tasksvariable in theINDEX_COMPLETEmessage and error handling. While this code is unchanged, it's crucial to ensure that it correctly uses the potentially modifiedtasksarray.Please run the following script to confirm the correct usage of
tasks:✅ Verification successful
Verified: The
tasksvariable is correctly used in theINDEX_COMPLETEmessage and error handling as per the provided code snippets.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the modified `tasks` variable is used correctly in the subsequent code. # Test: Check if `tasks` is used directly after the modification block rg --type typescript -A 10 -B 2 'if \(typeof config\.maxDuration === "number"\) \{' packages/cli-v3/src/entryPoints/dev-index-worker.ts # Test: Verify usage of `tasks` in the `INDEX_COMPLETE` message rg --type typescript -A 10 -B 2 '"INDEX_COMPLETE",' packages/cli-v3/src/entryPoints/dev-index-worker.ts # Test: Check usage of `tasks` in error handling rg --type typescript -A 10 -B 2 '"TASKS_FAILED_TO_PARSE",' packages/cli-v3/src/entryPoints/dev-index-worker.tsLength of output: 461
Script:
#!/bin/bash # Description: Verify that the modified `tasks` variable is used correctly in the subsequent code. # Test: Check if `tasks` is used directly after the modification block using the correct file type. rg --type ts -A 10 -B 2 'if \(typeof config\.maxDuration === "number"\) \{' packages/cli-v3/src/entryPoints/dev-index-worker.ts # Test: Verify usage of `tasks` in the `INDEX_COMPLETE` message with corrected file type. rg --type ts -A 10 -B 2 '"INDEX_COMPLETE",' packages/cli-v3/src/entryPoints/dev-index-worker.ts # Test: Check usage of `tasks` in error handling with corrected file type. rg --type ts -A 10 -B 2 '"TASKS_FAILED_TO_PARSE",' packages/cli-v3/src/entryPoints/dev-index-worker.tsLength of output: 1407
packages/cli-v3/src/entryPoints/deploy-index-worker.ts (1)
118-130: 🛠️ Refactor suggestionImplement type safety and add validation for
maxDurationThe implementation of the global
maxDurationconfiguration aligns well with the PR objectives. However, consider the following improvements:
- Enhance type safety by using a type guard or assertion for
config.maxDuration.- Add validation to ensure
maxDurationis a positive number.- Consider adding in-line comments to explain the purpose of this block.
Here's a suggested implementation with these improvements:
// Apply global maxDuration to tasks without a specific maxDuration if (typeof config.maxDuration === "number" && config.maxDuration > 0) { tasks = tasks.map((task) => { if (typeof task.maxDuration !== "number") { return { ...task, maxDuration: config.maxDuration, }; } return task; }); } else if (config.maxDuration !== undefined) { console.warn("Invalid global maxDuration. It should be a positive number."); }This implementation includes a warning for invalid
maxDurationvalues and ensures that only positive numbers are applied.To ensure that
maxDurationis being used correctly throughout the codebase, let's run a verification script:This script will help us understand how
maxDurationis used and validated across the project.apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (5)
11-11: LGTM: StopIcon import added.The
StopIconimport has been correctly added, which will likely be used for the new "TIMED_OUT" status.
145-146: LGTM: TIMED_OUT case added to TaskRunStatusIcon.The new case for "TIMED_OUT" has been correctly implemented, returning a StopIcon component with the appropriate class names.
181-182: LGTM: TIMED_OUT case added to runStatusClassNameColor.The new case for "TIMED_OUT" has been correctly implemented, returning "text-error". This is consistent with other error-like statuses in the function.
219-220: LGTM: TIMED_OUT case added to runStatusTitle.The new case for "TIMED_OUT" has been correctly implemented, returning "Timed out". This title is concise and accurately describes the status.
Line range hint
1-224: Summary: TIMED_OUT status successfully implemented.The new "TIMED_OUT" status has been successfully implemented across all relevant parts of the file:
- Import of the StopIcon
- Addition to taskRunStatusDescriptions
- New case in TaskRunStatusIcon
- New case in runStatusClassNameColor
- New case in runStatusTitle
These changes are consistent and align well with the existing code structure. The implementation will allow the UI to properly display and handle tasks that have exceeded their maximum duration.
To ensure completeness:
This script will help ensure that "TIMED_OUT" is properly added to all necessary arrays and type definitions within the file.
✅ Verification successful
✅ TIMED_OUT status fully integrated.
The
TIMED_OUTstatus has been successfully added to all necessary arrays and type definitions withinTaskRunStatus.tsx:
- Description Mapping: Added to
taskRunStatusDescriptions.- Icon Rendering: Included as a case in
TaskRunStatusIcon.- Class Name Coloring: Handled in
runStatusClassNameColor.- Status Title Generation: Managed in
runStatusTitle.These additions ensure that the UI will accurately display and handle tasks that have exceeded their maximum duration without any inconsistencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that TIMED_OUT is added to all necessary arrays and type definitions # Check if TIMED_OUT is added to allTaskRunStatuses and filterableTaskRunStatuses grep -n 'TIMED_OUT' apps/webapp/app/components/runs/v3/TaskRunStatus.tsx # Verify TaskRunStatus type includes TIMED_OUT grep -n 'TaskRunStatus' apps/webapp/app/components/runs/v3/TaskRunStatus.tsxLength of output: 1269
apps/webapp/app/v3/services/createTaskRunAttempt.server.ts (1)
Line range hint
1-199: Verify the usage ofmaxDurationin the task execution system.The addition of the
maxDurationproperty to theexecution.runobject is well-integrated and doesn't introduce any apparent issues. It provides the necessary information for implementing themaxDurationfeature as described in the PR objectives.To ensure the complete implementation of the feature, please verify that the task execution system correctly utilizes this
maxDurationproperty to abort tasks that exceed their maximum duration. You may want to check the following:
- The component responsible for task execution.
- Any middleware or interceptors that monitor task duration.
- Error handling and reporting mechanisms for aborted tasks.
Run the following script to find potential usage of
maxDurationin the codebase:packages/core/src/v3/schemas/common.ts (2)
95-95: LGTM: Addition of MAX_DURATION_EXCEEDED error codeThe new error code
MAX_DURATION_EXCEEDEDis a good addition that aligns with the PR objective of introducing amaxDurationfeature for tasks. It follows the existing naming convention and will be useful for error handling and debugging when tasks exceed their maximum duration.
116-116: LGTM: Addition of MAX_DURATION_EXCEEDED to TaskRunInternalError schemaThe inclusion of
MAX_DURATION_EXCEEDEDin theTaskRunInternalErrorschema's enum is consistent with its addition toTaskRunErrorCodes. This ensures that the new error type can be properly handled internally, maintaining the integrity of the error handling system.apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (2)
Line range hint
1-320: Summary: "TIMED_OUT" status successfully implementedThe changes in this file successfully implement the "TIMED_OUT" status to support the new
maxDurationfeature for tasks. The implementation is minimal, focused, and consistent with the existing code structure.Key points:
- The
apiStatusToRunStatusesmethod now handles the "TIMED_OUT" status correctly.- No other parts of the file required changes, maintaining simplicity.
- The changes align well with the PR objectives.
To ensure full compatibility, make sure to:
- Update any relevant tests to cover the new "TIMED_OUT" status.
- Verify that the API documentation is updated to reflect this new status.
- Check if any frontend components need to be updated to display the "TIMED_OUT" status appropriately.
Line range hint
40-307: Verify "TIMED_OUT" status propagation in API responseThe
callmethod looks good and doesn't require changes to support the new "TIMED_OUT" status. However, it's important to ensure that this new status is properly propagated to the API response.Please run the following script to verify that the "TIMED_OUT" status is correctly included in the API response:
If the "TIMED_OUT" status is not properly handled in the
ApiRetrieveRunPresenter, you may need to update it to support the new status.apps/webapp/app/presenters/v3/SpanPresenter.server.ts (3)
72-72: LGTM: Addition ofmaxDurationInSecondsfieldThe addition of the
maxDurationInSecondsfield to the select statement is correct and aligns with the PR objective of introducing amaxDurationfeature for tasks.
Line range hint
1-365: Summary: Implementation ofmaxDurationfeatureThe changes in this file consistently implement the new
maxDurationfeature as per the PR objectives. The modifications include:
- Adding
maxDurationInSecondsto the database query.- Incorporating
maxDurationinto the context object.- Exposing
maxDurationInSecondsin the return object of thegetRunmethod.These changes appear to be complete and consistent. Minor suggestions for improvement have been made in the individual comments.
To ensure full implementation:
- Verify that all consumers of the
getRunmethod are updated to handle the newmaxDurationInSecondsfield.- Confirm that the
maxDurationfeature is properly utilized in the task execution logic (which might be in a different file).- Ensure that appropriate error handling and user feedback mechanisms are in place for when a task exceeds its
maxDuration.To check for any missed implementations or inconsistencies:
Please review the results to ensure comprehensive implementation of the
maxDurationfeature across the codebase.
312-312: LGTM: Exposure ofmaxDurationInSecondsin return objectThe addition of
maxDurationInSecondsto the return object of thegetRunmethod is correct and consistent with the PR objective.To ensure consistency across the codebase, let's verify that all consumers of the
getRunmethod are updated to handle this new field:Please review the results to ensure that all relevant code paths are updated to handle the new
maxDurationInSecondsfield.apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1)
Line range hint
1-458: Verify complete implementation ofmaxDurationfeatureWhile this change correctly adds support for the "TIMED_OUT" status, it appears to be only part of the complete
maxDurationfeature implementation. The PR objectives mention configuringmaxDurationin different contexts (global default, task-specific, on-the-fly override), which are not addressed in this file.Please ensure that:
- The
maxDurationcan be configured intrigger.config.tsas a global default.- Individual tasks can define their own
maxDuration.- The
triggercommand supports specifying amaxDurationthat overrides other settings.- Appropriate tests are added to cover these new functionalities.
Consider running the following script to verify the implementation:
This script will help identify if the
maxDurationfeature has been implemented across different parts of the codebase as specified in the PR objectives.packages/core/src/v3/errors.ts (2)
Line range hint
1-65: Summary of changes in errors.tsThe modifications in this file enhance the handling of internal errors by providing more detailed error information. While these changes don't directly implement the
maxDurationfeature mentioned in the PR objectives, they likely support improved error reporting for that feature.Overall, the changes are well-implemented and contribute to better error handling. However, it would be beneficial to provide more context on how these changes relate to the
maxDurationfeature and to include comments explaining the purpose of these modifications for future maintainers.
Line range hint
1-65: Clarify the relationship between these changes and the maxDuration feature.While these changes improve error handling for internal errors, they don't directly implement the
maxDurationfeature mentioned in the PR objectives. It would be helpful to clarify how these error handling improvements relate to themaxDurationfeature.Could you provide more context on how these error handling changes support the
maxDurationfeature? Are there other files in this PR that implement the core functionality?✅ Verification successful
✅ Verification Successful: Changes Support the
maxDurationFeatureThe modifications in
packages/core/src/v3/errors.tsenhance error handling related to themaxDurationfeature. The shell script results show thatmaxDurationis implemented across multiple files, and these error handling improvements are integral to its functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for maxDuration-related changes in other files rg --type typescript -i 'maxDuration'Length of output: 72
Script:
#!/bin/bash # Search for maxDuration-related changes in TypeScript files rg -g "*.ts" -g "*.tsx" -i 'maxDuration'Length of output: 6963
apps/webapp/app/v3/services/createBackgroundWorker.server.ts (2)
Line range hint
1-465: Summary: Successfully implementedmaxDurationfeatureThe changes in this file successfully implement the
maxDurationfeature for tasks as described in the PR objectives. The implementation:
- Adds a
maxDurationInSecondsproperty to the task creation process.- Correctly handles both the presence and absence of
task.maxDuration.- Ensures a minimum duration of 5 seconds to prevent extremely short durations.
The changes are well-integrated into the existing code structure and don't introduce any apparent issues. However, to ensure full functionality of this feature:
- Verify that task execution components are updated to respect the new
maxDurationInSecondsproperty.- Update relevant API endpoints or interfaces for task creation/update.
- Modify documentation to reflect this new feature.
Overall, this implementation provides a solid foundation for the
maxDurationfeature in tasks.
159-159: Verify usage ofmaxDurationInSecondsin related componentsThe addition of
maxDurationInSecondsto the task creation process is well-implemented. To ensure full functionality:
- Verify that components or services responsible for task execution are updated to respect this new
maxDurationInSecondsproperty.- Ensure that any API endpoints or interfaces for task creation/update are modified to accept the
maxDurationparameter.- Update relevant documentation to reflect this new feature.
To check for potential usage or necessary updates, run the following script:
✅ Verification successful
maxDurationInSecondscorrectly integrated across the codebaseThe addition of
maxDurationInSecondsincreateBackgroundWorker.server.tsis consistent with its usage in other components and services. No issues were found related to this change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for potential places where maxDurationInSeconds should be used or updated # Search for task execution related files echo "Searching for task execution related files:" rg --type typescript --type javascript -l 'executeTask|runTask|taskExecution' # Search for API endpoints or interfaces related to task creation/update echo "\nSearching for API endpoints or interfaces for task creation/update:" rg --type typescript --type javascript -l 'createTask|updateTask|TaskCreationDto|TaskUpdateDto' # Search for existing usage of maxDurationInSeconds echo "\nSearching for existing usage of maxDurationInSeconds:" rg --type typescript --type javascript 'maxDurationInSeconds'Length of output: 711
Script:
#!/bin/bash # Description: Search for potential places where maxDurationInSeconds should be used or updated # Define file extensions to include FILE_EXTENSIONS="*.ts *.tsx *.js *.jsx" # Search for task execution related files echo "Searching for task execution related files:" rg -l 'executeTask|runTask|taskExecution' --glob '{'"$FILE_EXTENSIONS"'}' # Search for API endpoints or interfaces related to task creation/update echo "\nSearching for API endpoints or interfaces for task creation/update:" rg -l 'createTask|updateTask|TaskCreationDto|TaskUpdateDto' --glob '{'"$FILE_EXTENSIONS"'}' # Search for existing usage of maxDurationInSeconds echo "\nSearching for existing usage of maxDurationInSeconds:" rg 'maxDurationInSeconds' --glob '{'"$FILE_EXTENSIONS"'}'Length of output: 1086
Script:
#!/bin/bash # Description: Retry searching for potential places where maxDurationInSeconds should be used or updated without restrictive glob patterns # Search for task execution related files echo "Searching for task execution related files:" rg -l 'executeTask|runTask|taskExecution' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -u # Search for API endpoints or interfaces related to task creation/update echo "\nSearching for API endpoints or interfaces for task creation/update:" rg -l 'createTask|updateTask|TaskCreationDto|TaskUpdateDto' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -u # Search for existing usage of maxDurationInSeconds echo "\nSearching for existing usage of maxDurationInSeconds:" rg 'maxDurationInSeconds' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -uLength of output: 3956
apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (1)
381-382: Clarify the enforcement of maxDurationInSecondsThe implementation of
maxDurationInSecondsis correct and aligns with the PR objectives. However, the actual enforcement of this maximum duration is not visible in this file.Could you please clarify how the
maxDurationInSecondsis enforced? Is it handled in the background worker or in another part of the system? It would be helpful to add a comment or link to the relevant code that handles the task abortion when the maximum duration is exceeded.To verify the implementation, you can run the following script to search for usage of
maxDurationInSeconds:This will help identify where the maximum duration is being used and potentially enforced.
packages/core/src/v3/schemas/api.ts (2)
447-448: LGTM! The new status aligns with themaxDurationfeature.The addition of the
TIMED_OUTstatus to theRunStatusenum is appropriate and well-documented. It complements the newmaxDurationfeature and provides a clear way to identify tasks that have exceeded their maximum allowed duration.
87-87: Verify complete implementation ofmaxDurationfeatureThe changes in this file correctly implement the schema for the
maxDurationfeature and the correspondingTIMED_OUTstatus. To ensure full functionality, it's important to verify that other parts of the codebase properly handle these new additions.Please run the following script to check for proper implementation of the
maxDurationfeature:This script will help identify where the
maxDurationfeature andTIMED_OUTstatus are being used or handled in the codebase, and if there are any remaining TODOs related to this feature.Also applies to: 447-448
✅ Verification successful
Retrying verification with adjusted search parameters
The previous script encountered an error due to an unrecognized file type. Please run the updated script below to check for proper implementation of the
maxDurationfeature without specifying the file type:This updated script removes the
--type typescriptflag, allowingrgto search through all file types in the repository.
maxDurationFeature Fully VerifiedThe
maxDurationfeature and its associatedTIMED_OUTstatus are correctly implemented across the codebase. No additional TODOs or pending tasks related to this feature were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for proper implementation of maxDuration feature # Test 1: Search for maxDuration usage echo "Searching for maxDuration usage:" rg --type typescript 'maxDuration' # Test 2: Search for TIMED_OUT status handling echo "Searching for TIMED_OUT status handling:" rg --type typescript 'TIMED_OUT' # Test 3: Check for any TODOs related to maxDuration or TIMED_OUT echo "Checking for related TODOs:" rg --type typescript 'TODO.*(maxDuration|TIMED_OUT)'Length of output: 458
Script:
# #!/bin/bash # Description: Check for proper implementation of maxDuration feature without specifying file type # Test 1: Search for maxDuration usage echo "Searching for maxDuration usage:" rg 'maxDuration' # Test 2: Search for TIMED_OUT status handling echo "Searching for TIMED_OUT status handling:" rg 'TIMED_OUT' # Test 3: Check for any TODOs related to maxDuration or TIMED_OUT echo "Checking for related TODOs:" rg 'TODO.*(maxDuration|TIMED_OUT)'Length of output: 11593
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.v3.$projectParam.runs.$runParam.spans.$spanParam/route.tsx (1)
680-685: LGTM! New "Max duration" property added.The addition of the "Max duration" property item is well-implemented and consistent with the existing code structure. It correctly displays the
maxDurationInSecondsvalue from therunobject, showing "–" when the value is not available.A few observations:
- The formatting is consistent with other property items.
- The value is displayed in seconds, which is appropriate for this context.
- The conditional rendering ensures that no value is shown when
maxDurationInSecondsis falsy.To ensure this change is properly integrated, let's verify if the
maxDurationInSecondsproperty is being set correctly in the run object:✅ Verification successful
To proceed with the verification, please run the following corrected shell scripts:
Verification Successful:
maxDurationInSecondsis correctly set and utilized.The
maxDurationInSecondsproperty is consistently assigned across relevant services and presenters, ensuring that the "Max duration" display functions as intended. No issues were found regarding its implementation or integration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if maxDurationInSeconds is being set in relevant files # Search for maxDurationInSeconds in TypeScript/JavaScript files rg --type ts --type js 'maxDurationInSeconds' # Search for maxDuration in TypeScript/JavaScript files (in case it's named differently before reaching this component) rg --type ts --type js 'maxDuration'Length of output: 301
Script:
#!/bin/bash # Description: Check if maxDurationInSeconds is being set in relevant files # Search for maxDurationInSeconds in TypeScript and JavaScript files rg --type ts --type js 'maxDurationInSeconds' # Search for maxDuration in TypeScript and JavaScript files (in case it's named differently before reaching this component) rg --type ts --type js 'maxDuration'Length of output: 8192
packages/database/prisma/schema.prisma (1)
1839-1840: LGTM! The new status is well-defined and documented.The addition of the
TIMED_OUTstatus to theTaskRunStatusenum is appropriate and consistent with the introduction of themaxDurationfeature. The comment above the new status clearly explains its purpose.packages/core/src/v3/timeout/usageTimeoutManager.ts (3)
1-2: LGTM!The imports are correctly specified and follow the proper naming conventions.
26-28: LGTM!The
TaskRunExceededMaxDurationerror is correctly instantiated with the expected parameters, providing clear information about the timeout duration and the actual CPU time used.
16-34: Verify the behavior whentimeoutInSecondsis zero or negative.The current implementation does not handle the case when
timeoutInSecondsis zero or negative. It's important to consider the expected behavior in such scenarios and handle them appropriately.Here's a script to verify the behavior:
If the above tests indicate potential issues, consider adding validation and error handling for the
timeoutInSecondsparameter to ensure proper behavior and prevent unexpected results.Do you want me to generate the code for validating and handling invalid
timeoutInSecondsvalues?packages/core/src/v3/timeout/api.ts (8)
1-2: LGTM!The imports are correctly specified and follow the proper syntax.
6-10: LGTM!The
NoopTimeoutManagerclass correctly implements theTimeoutManagerinterface and provides a no-op implementation for theabortAfterTimeoutmethod.
12-12: LGTM!The
NOOP_TIMEOUT_MANAGERconstant is correctly initialized with an instance of theNoopTimeoutManagerclass.
14-46: LGTM!The
TimeoutAPIclass correctly implements theTimeoutManagerinterface and provides the following functionality:
- Singleton pattern using a private constructor and a static
getInstancemethod.- Getter for the
signalproperty that retrieves the signal from the current timeout manager.abortAfterTimeoutmethod that delegates to the current timeout manager.setGlobalManagermethod to register a global timeout manager.disablemethod to unregister the global timeout manager.- Private
#getManagerManagermethod to retrieve the current timeout manager or fall back to theNOOP_TIMEOUT_MANAGER.The class is well-structured and follows best practices for implementing a singleton and managing global state.
🧰 Tools
🪛 Biome
[error] 20-20: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 21-21: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 24-24: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
31-33: LGTM!The
abortAfterTimeoutmethod correctly delegates to the current timeout manager using the#getManagerManagermethod.
35-37: LGTM!The
setGlobalManagermethod correctly registers the provided timeout manager using theregisterGlobalutility function.
39-41: LGTM!The
disablemethod correctly unregisters the global timeout manager using theunregisterGlobalutility function.
43-45: LGTM!The
#getManagerManagermethod correctly retrieves the current timeout manager using thegetGlobalutility function and falls back to theNOOP_TIMEOUT_MANAGERif no manager is registered.references/hello-world/src/trigger/example.ts (1)
46-53: LGTM:maxDurationTaskis well-implementedThe
maxDurationTaskfunction correctly handles the sleep duration with cancellation support and returns the current usage as expected.packages/core/src/v3/tracer.ts (5)
61-62: LGTM!The addition of the optional
signalparameter of typeAbortSignalto thestartActiveSpanmethod signature is a valid change to support abort functionality.
68-69: LGTM!Introducing the
spanEndedvariable to track the state of the span is a good approach to prevent redundant operations on an already ended span.
79-86: LGTM!Adding an event listener to the
signalparameter to handle abort events is a correct implementation. The listener correctly checks thespanEndedstate before recording the exception and ending the span to avoid duplicate actions.
109-115: LGTM!The error handling logic has been appropriately updated to check the
spanEndedstate before recording exceptions and setting the span status to error. This prevents redundant error handling for an already ended span.
119-134: LGTM!The finalization logic in the
finallyblock has been correctly wrapped with a check for thespanEndedstate. This ensures that the span is ended and usage measurement attributes are set only if the span has not already ended, preventing duplicate span endings.packages/cli-v3/src/entryPoints/dev-run-worker.ts (4)
81-81: Verify theUsageTimeoutManageris used correctly.The
UsageTimeoutManageris instantiated with thedevUsageManagerand set as the global timeout manager. Ensure that it is correctly utilized throughout the codebase and that there are no conflicting timeout manager instances.Run the following script to verify the
UsageTimeoutManagerusage:#!/bin/bash # Description: Verify the `UsageTimeoutManager` is used correctly. # Test 1: Search for the instantiation of `UsageTimeoutManager`. Expect: Only 1 occurrence. rg --type typescript -w 'new UsageTimeoutManager' # Test 2: Search for the setting of global timeout manager. Expect: Only 1 occurrence. rg --type typescript -w 'timeout.setGlobalManager'
278-315: Verify the task execution timeout logic.The changes introduce a new timeout mechanism for task execution using the
UsageTimeoutManager. The key points to verify are:
- The
signalis correctly created based on theexecution.run.maxDurationvalue.- The abort event listener is correctly attached to the
signaland handles the timeout scenario appropriately.- The
signalis correctly passed to theexecutor.executemethod.Run the following script to verify the timeout logic:
#!/bin/bash # Description: Verify the task execution timeout logic. # Test 1: Search for the creation of the `signal`. Expect: Only 1 occurrence. rg --type typescript -w 'timeout.abortAfterTimeout' # Test 2: Search for the abort event listener. Expect: Only 1 occurrence. rg --type typescript -w 'signal\?\.addEventListener\("abort"' # Test 3: Search for the passing of `signal` to `executor.execute`. Expect: Only 1 occurrence. rg --type typescript -w 'executor\.execute\(.*signal'
319-329: Verify the task completion logic.The changes introduce a check to ensure that the task completion message is sent only if the task is still running. This prevents premature completion notifications in case the task is aborted due to timeout.
Run the following script to verify the completion logic:
#!/bin/bash # Description: Verify the task completion logic. # Test: Search for the conditional sending of completion message. Expect: Only 1 occurrence. rg --type typescript -w 'if \(_isRunning\) \{'
17-17: Verify the new imports are used correctly.The new imports
timeoutandUsageTimeoutManagerare used later in the code to manage task execution timeouts. Ensure that these imports are correctly utilized and that there are no unused imports.Run the following script to verify the imports:
Also applies to: 23-23
✅ Verification successful
Imports
timeoutandUsageTimeoutManagerare used correctly and there are no unused imports.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new imports are used correctly. # Test 1: Search for the usage of `timeout`. Expect: At least 1 occurrence. rg --type typescript -w timeout # Test 2: Search for the usage of `UsageTimeoutManager`. Expect: At least 1 occurrence. rg --type typescript -w UsageTimeoutManagerLength of output: 149
Script:
#!/bin/bash # Description: Verify the new imports are used correctly by searching TypeScript files. # Test 1: Search for the usage of `timeout`. Expect: At least 1 occurrence. rg -g "*.ts" -g "*.tsx" -w timeout # Test 2: Search for the usage of `UsageTimeoutManager`. Expect: At least 1 occurrence. rg -g "*.ts" -g "*.tsx" -w UsageTimeoutManagerLength of output: 17363
packages/cli-v3/src/entryPoints/deploy-run-worker.ts (6)
87-95: Verify the initialization of theUsageTimeoutManager.The
UsageTimeoutManageris initialized with an instance ofDevUsageManagerand set as the global timeout manager usingtimeout.setGlobalManager().Ensure that:
- The
DevUsageManageris properly initialized with the correct configuration.- The
UsageTimeoutManagerconstructor accepts an instance ofDevUsageManager.- The
timeout.setGlobalManager()function is properly exported and accepts an instance ofUsageTimeoutManager.#!/bin/bash # Description: Verify the initialization of the `UsageTimeoutManager`. # Test 1: Search for the initialization of `DevUsageManager`. # Expect: The `DevUsageManager` is initialized without any arguments. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'new DevUsageManager\(\)' ./packages/core-v3/src/workers # Test 2: Search for the constructor of `UsageTimeoutManager`. # Expect: The constructor accepts an instance of a usage manager (e.g., `DevUsageManager`). ast-grep --lang typescript --pattern $'class UsageTimeoutManager { constructor($_) { $$$ } }' ./packages/core-v3/src/workers # Test 3: Search for the `setGlobalManager` function in the `timeout` module. # Expect: The function is exported and accepts an instance of `UsageTimeoutManager`. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'export (function|const) setGlobalManager' ./packages/core-v3/src
308-340: Verify the creation and handling of the timeout signal.The code introduces a timeout signal that aborts the task execution if it exceeds the specified
maxDuration. The signal is created usingtimeout.abortAfterTimeout()and listens for the "abort" event to handle the timeout.Ensure that:
- The
timeout.abortAfterTimeout()function is properly exported and returns anAbortSignalthat aborts after the specified duration.- The "abort" event is fired when the timeout is exceeded.
- The task execution is properly aborted and the completion message is sent with the appropriate error details when the timeout is exceeded.
#!/bin/bash # Description: Verify the creation and handling of the timeout signal. # Test 1: Search for the `abortAfterTimeout` function in the `timeout` module. # Expect: The function is exported and returns an `AbortSignal`. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'export (function|const) abortAfterTimeout' ./packages/core-v3/src # Test 2: Search for the usage of the "abort" event on the timeout signal. # Expect: The "abort" event is listened to and handled appropriately. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'signal\?\.addEventListener\("abort", \(?.*\)? => {' ./packages/cli-v3/src/entryPoints/deploy-run-worker.ts # Test 3: Search for the sending of the completion message when the timeout is exceeded. # Expect: The completion message is sent with the appropriate error details. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'sender\.send\("TASK_RUN_COMPLETED", {.*error: {.*code: TaskRunErrorCodes\.MAX_DURATION_EXCEEDED' ./packages/cli-v3/src/entryPoints/deploy-run-worker.ts
33-33: Verify the usage of theUsageTimeoutManagerclass.The
UsageTimeoutManagerclass is imported from@trigger.dev/core/v3/workers. Ensure that this class is properly exported from the@trigger.dev/core/v3/workersmodule and contains the necessary functionality for managing task execution timeouts based on usage.#!/bin/bash # Description: Verify the `UsageTimeoutManager` class is exported from `@trigger.dev/core/v3/workers`. # Test: Search for the `UsageTimeoutManager` export in the `@trigger.dev/core/v3/workers` module. # Expect: The `UsageTimeoutManager` class is exported. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'export (class|interface) UsageTimeoutManager' ./packages/core-v3/src/workers
351-361: Verify the sending of the completion message only if the worker is still running.The completion message is sent only if the
_isRunningflag istrue, indicating that the worker is still running. This ensures that the completion message is not sent multiple times if the task execution is aborted due to a timeout.Ensure that:
- The
_isRunningflag is properly set tofalsewhen the task execution is aborted due to a timeout.- The completion message is sent only if
_isRunningistrue.#!/bin/bash # Description: Verify the sending of the completion message only if the worker is still running. # Test 1: Search for the setting of `_isRunning` to `false` when the task execution is aborted. # Expect: `_isRunning` is set to `false` in the "abort" event handler. rg --type typescript -g '!.*test.*' -g '!.*spec.*' '_isRunning = false' ./packages/cli-v3/src/entryPoints/deploy-run-worker.ts # Test 2: Search for the conditional sending of the completion message based on `_isRunning`. # Expect: The completion message is sent only if `_isRunning` is `true`. rg --type typescript -g '!.*test.*' -g '!.*spec.*' 'if \(_isRunning\) {.*sender\.send\("TASK_RUN_COMPLETED"' ./packages/cli-v3/src/entryPoints/deploy-run-worker.ts
341-347: Verify the passing of the timeout signal to theexecutor.execute()function.The timeout signal is passed as an argument to the
executor.execute()function.Ensure that:
- The
execute()function of theTaskExecutorclass accepts anAbortSignalas an argument.- The
execute()function properly handles the aborted signal and terminates the task execution gracefully.#!/bin/bash # Description: Verify the passing of the timeout signal to the `executor.execute()` function. # Test 1: Search for the `execute` function in the `TaskExecutor` class. # Expect: The function accepts an `AbortSignal` as an argument. ast-grep --lang typescript --pattern $'class TaskExecutor { $$$ execute($_, $_, $_, $_, $_) { $$$ } }' ./packages/core-v3/src/workers # Test 2: Search for the handling of the aborted signal in the `execute()` function. # Expect: The function checks for the aborted signal and terminates the task execution gracefully. rg --type typescript -g '!.*test.*' -g '!.*spec.*' '(signal|abortSignal)\.aborted' ./packages/core-v3/src/workers
17-17: Verify the usage of thetimeoutmodule.The
timeoutmodule is imported from@trigger.dev/core/v3. Ensure that this module is properly exported from the@trigger.dev/core/v3package and contains the necessary functionality for managing task execution timeouts.packages/core/src/v3/workers/taskExecutor.ts (36)
60-61: LGTM!The addition of the
signalparameter to theexecutemethod signature is appropriate for enabling cancellation support.
213-214: LGTM!The
signalparameter is being passed to thestartActiveSpanmethod of the tracer, which is the correct way to propagate the cancellation signal to the span.
220-220: LGTM!The
signalparameter is being added to the#callRunmethod signature, which is consistent with the changes made in theexecutemethod.
229-229: LGTM!The
signalparameter is being passed to therunFnfunction, which allows the task's run function to handle cancellation.
232-236: LGTM!The
signalparameter is being passed to themiddlewareFnfunction and therunFnfunction within thenextcallback, which allows the task's middleware and run functions to handle cancellation.
239-240: LGTM!The
signalparameter is being added to the#callInitFunctionsmethod signature and is being passed to the#callConfigInitmethod, which is consistent with the changes made in theexecutemethod.
251-251: LGTM!The
signalparameter is being passed to theinitFnfunction, which allows the task's init function to handle cancellation.
261-261: LGTM!The
signalparameter is being added to the#callConfigInitmethod signature, which is consistent with the changes made in theexecutemethod.
271-271: LGTM!The
signalparameter is being passed to theinitFnfunction from the imported config, which allows the config's init function to handle cancellation.
285-286: LGTM!The
signalparameter is being added to the#callOnSuccessFunctionsmethod signature, which is consistent with the changes made in theexecutemethod.
294-295: LGTM!The
signalparameter is being passed to the#callOnSuccessFunctionmethod for the task's onSuccess function, which allows it to handle cancellation.
304-305: LGTM!The
signalparameter is being passed to the#callOnSuccessFunctionmethod for the config's onSuccess function, which allows it to handle cancellation.
315-316: LGTM!The
signalparameter is being added to the#callOnSuccessFunctionmethod signature, which is consistent with the changes made in the#callOnSuccessFunctionsmethod.
326-326: LGTM!The
signalparameter is being passed to theonSuccessFnfunction, which allows the onSuccess function to handle cancellation.
343-344: LGTM!The
signalparameter is being added to the#callOnFailureFunctionsmethod signature, which is consistent with the changes made in theexecutemethod.
352-353: LGTM!The
signalparameter is being passed to the#callOnFailureFunctionmethod for the task's onFailure function, which allows it to handle cancellation.
362-363: LGTM!The
signalparameter is being passed to the#callOnFailureFunctionmethod for the config's onFailure function, which allows it to handle cancellation.
373-374: LGTM!The
signalparameter is being added to the#callOnFailureFunctionmethod signature, which is consistent with the changes made in the#callOnFailureFunctionsmethod.
384-384: LGTM!The
signalparameter is being passed to theonFailureFnfunction, which allows the onFailure function to handle cancellation.
397-397: LGTM!The
signalparameter is being added to the#callOnStartFunctionsmethod signature, which is consistent with the changes made in theexecutemethod.
403-404: LGTM!The
signalparameter is being passed to the#callOnStartFunctionmethod for the config's onStart function, which allows it to handle cancellation.
407-414: LGTM!The
signalparameter is being passed to the#callOnStartFunctionmethod for the task's onStart function, which allows it to handle cancellation.
422-423: LGTM!The
signalparameter is being added to the#callOnStartFunctionmethod signature, which is consistent with the changes made in the#callOnStartFunctionsmethod.
433-433: LGTM!The
signalparameter is being passed to theonStartFnfunction, which allows the onStart function to handle cancellation.
446-451: LGTM!The
signalparameter is being added to the#callTaskCleanupmethod signature, which is consistent with the changes made in theexecutemethod.
459-459: LGTM!The
signalparameter is being passed to thecleanupFnfunction, which allows the task's cleanup function to handle cancellation.
467-468: LGTM!The
signalparameter is being added to the#handleErrormethod signature, which is consistent with the changes made in theexecutemethod.
520-520: LGTM!The
signalparameter is being passed to the task'shandleErrorfunction, which allows it to handle cancellation.
528-528: LGTM!The
signalparameter is being passed to the imported config'shandleErrorfunction, which allows it to handle cancellation.
168-169: Verify thesignalparameter is handled correctly in#callOnFailureFunctions.The
signalparameter is being passed to the#callOnFailureFunctionsmethod. Ensure that thesignalis properly handled within the implementation of#callOnFailureFunctionsto support cancellation.#!/bin/bash # Description: Verify the `signal` parameter is handled correctly in `#callOnFailureFunctions`. # Test: Search for the `#callOnFailureFunctions` method. Expect: The method should accept and handle the `signal` parameter. ast-grep --lang typescript --pattern $'async #callOnFailureFunctions(payload: unknown, error: unknown, ctx: TaskRunContext, initOutput: any, signal?: AbortSignal) { $$$ }'
203-203: Verify thesignalparameter is handled correctly in#callTaskCleanup.The
signalparameter is being passed to the#callTaskCleanupmethod. Ensure that thesignalis properly handled within the implementation of#callTaskCleanupto support cancellation.#!/bin/bash # Description: Verify the `signal` parameter is handled correctly in `#callTaskCleanup`. # Test: Search for the `#callTaskCleanup` method. Expect: The method should accept and handle the `signal` parameter. ast-grep --lang typescript --pattern $'async #callTaskCleanup(payload: unknown, ctx: TaskRunContext, init: unknown, signal?: AbortSignal) { $$$ }'
157-158: Verify thesignalparameter is handled correctly in#handleError.The
signalparameter is being passed to the#handleErrormethod. Ensure that thesignalis properly handled within the implementation of#handleErrorto support cancellation.
104-104: Verify thesignalparameter is handled correctly in#callRun.The
signalparameter is being passed to the#callRunmethod. Ensure that thesignalis properly handled within the implementation of#callRunto support cancellation.
102-102: Verify thesignalparameter is handled correctly in#callInitFunctions.The
signalparameter is being passed to the#callInitFunctionsmethod. Ensure that thesignalis properly handled within the implementation of#callInitFunctionsto support cancellation.
106-106: Verify thesignalparameter is handled correctly in#callOnSuccessFunctions.The
signalparameter is being passed to the#callOnSuccessFunctionsmethod. Ensure that thesignalis properly handled within the implementation of#callOnSuccessFunctionsto support cancellation.
99-99: Verify thesignalparameter is handled correctly in#callOnStartFunctions.The
signalparameter is being passed to the#callOnStartFunctionsmethod. Ensure that thesignalis properly handled within the implementation of#callOnStartFunctionsto support cancellation.packages/trigger-sdk/src/v3/shared.ts (7)
160-165: LGTM!The new
maxDurationproperty is added correctly to theTaskOptionstype with a clear comment explaining its purpose and minimum value.
512-520: LGTM!The new
maxDurationproperty is added correctly to theTaskRunOptionstype with a clear comment explaining its purpose and ability to override the task'smaxDurationsetting.
815-815: LGTM!The
maxDurationoption is correctly passed to the API client when triggering a task.
875-876: LGTM!The
maxDurationoption is correctly passed to the API client when batch triggering tasks.
940-941: LGTM!The
maxDurationoption is correctly passed to the API client when triggering a task and waiting for the result.
1034-1035: LGTM!The
maxDurationoption is correctly passed to the API client when batch triggering tasks and waiting for the results.
621-621: Verify the usage ofmaxDurationin the codebase.The
maxDurationproperty is correctly passed to theregisterTaskMetadatafunction when creating a task. However, it's important to ensure that this value is properly utilized throughout the system, such as in the task runner, to enforce the maximum duration limit.Run the following script to verify the usage of
maxDuration:apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (2)
1072-1072: Verify the addition ofmaxDurationto theProdTaskRunExecutiontype.The addition of the optional
maxDurationproperty to theProdTaskRunExecutiontype is correct. It aligns with the changes made to includemaxDurationInSecondsin the task run.Run the following script to verify the usage of
maxDurationin theProdTaskRunExecutiontype:#!/bin/bash # Description: Verify the usage of `maxDuration` in the `ProdTaskRunExecution` type. # Test: Search for the declaration of the `ProdTaskRunExecution` type. # Expect: The `maxDuration` property with the correct type and optionality. ast-grep --lang typescript --pattern $'type ProdTaskRunExecution = { $$$ maxDuration?: number; $$$ }'
406-407: Verify the usage ofmaxDurationInSecondsin the codebase.The addition of
maxDurationInSecondsto thelockedTaskRunupdate operation is correct. However, ensure that the rest of the codebase, particularly the execution logic, correctly handles and enforces this duration limit.Run the following script to verify the usage of
maxDurationInSeconds:
...abase/prisma/migrations/20241002155751_add_timed_out_status_to_task_run_status/migration.sql
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
references/hello-world/src/trigger/example.ts (1)
46-61: Approve implementation with a suggestion for error message improvementThe
maxDurationTaskimplementation looks good. It has a clear structure, proper use of thesignalparameter for cancellation support, and error throwing logic that aligns with the retry configuration.However, consider improving the error message to provide more context:
- throw new Error("Example error"); + throw new Error(`Task failed on attempt ${ctx.attempt.number}. Max attempts: 5`);This change will make debugging easier by providing more information about the current attempt and the maximum number of attempts allowed.
apps/webapp/app/v3/taskStatus.ts (1)
Line range hint
1-105: Consider additional updates to fully support "TIMED_OUT" status.The changes to add "TIMED_OUT" to FINAL_RUN_STATUSES and FAILED_RUN_STATUSES are correct and aligned with the PR objectives. However, to ensure full support for this new status, consider the following:
- Update any UI components that display task statuses to include "TIMED_OUT".
- Modify any analytics or logging systems to properly handle and report "TIMED_OUT" events.
- Update documentation to reflect the new "TIMED_OUT" status and its implications.
To help identify areas that might need updating, you can run the following script:
#!/bin/bash # Description: Identify potential areas that might need updating for "TIMED_OUT" status # Test: Search for files that handle task statuses rg --type typescript 'TaskRunStatus' # Test: Look for switch statements or if-else chains handling task statuses ast-grep --lang typescript --pattern 'switch ($_) { $$$case "COMPLETED_SUCCESSFULLY":$$$ case "COMPLETED_WITH_ERRORS":$$$ }' ast-grep --lang typescript --pattern 'if ($_ === "COMPLETED_SUCCESSFULLY" || $_ === "COMPLETED_WITH_ERRORS") { $$$ }' # Test: Check for any status-related enums or types that might need updating rg --type typescript 'enum.*TaskRunStatus' rg --type typescript 'type.*TaskRunStatus' # Test: Look for any UI components that might be displaying task statuses rg --type typescript 'render.*status' rg --type typescript '<.*Status'Please review the results of this script and update any relevant areas to fully support the new "TIMED_OUT" status.
packages/core/src/v3/schemas/api.ts (1)
87-87: LGTM! Consider adding a comment for clarity.The addition of the
maxDurationfield aligns well with the PR objectives. It's correctly implemented as an optional number field within theoptionsobject.Consider adding a brief comment to clarify the unit of measurement for
maxDuration(e.g., seconds, milliseconds). This would enhance code readability and prevent potential misunderstandings. For example:/** Maximum duration for task execution in milliseconds */ maxDuration: z.number().optional(),apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (1)
Line range hint
1-516: Summary: Excellent improvements to task run status display and interactionOverall, the changes in this file significantly enhance the user experience by providing more context and information about task run statuses. The additions align well with the PR objectives and improve the functionality of the
TaskRunsTablecomponent.Key improvements:
- Enhanced imports for better task run status handling.
- Informative tooltip added to the "Status" column header.
- Individual status displays now have tooltips with detailed descriptions.
These changes will make it easier for users to understand and interact with task run statuses. Great work!
Consider extracting the tooltip content generation for the "Status" column header into a separate component or function. This would improve code readability and make it easier to maintain or extend in the future.
🧰 Tools
🪛 Biome
[error] 140-140: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- apps/webapp/app/components/primitives/Select.tsx (1 hunks)
- apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (7 hunks)
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (3 hunks)
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1 hunks)
- apps/webapp/app/v3/services/completeAttempt.server.ts (1 hunks)
- apps/webapp/app/v3/taskStatus.ts (2 hunks)
- packages/core/src/v3/schemas/api.ts (2 hunks)
- references/hello-world/src/trigger/example.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/webapp/app/components/primitives/Select.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/webapp/app/components/runs/v3/TaskRunStatus.tsx
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
🧰 Additional context used
🪛 Biome
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
[error] 140-140: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (10)
references/hello-world/src/trigger/example.ts (3)
63-73: Address previous review comments and approve implementationThe
maxDurationParentTaskimplementation correctly adds themaxDurationfeature as described in the PR objectives. However, there are two improvements from previous review comments that should be addressed:
- Remove the unused
ctxparameter.- Add input validation for
sleepForandmaxDuration.Please apply the following changes:
export const maxDurationParentTask = task({ id: "max-duration-parent", - run: async (payload: { sleepFor?: number; maxDuration?: number }, { ctx, signal }) => { + run: async (payload: { sleepFor?: number; maxDuration?: number }, { signal }) => { + const sleepFor = payload.sleepFor ?? 10; + const maxDuration = payload.maxDuration ?? 600; + if (sleepFor <= 0 || maxDuration <= 0) { + throw new Error("sleepFor and maxDuration must be positive numbers"); + } const result = await maxDurationTask.triggerAndWait( - { sleepFor: payload.sleepFor ?? 10 }, - { maxDuration: payload.maxDuration ?? 600 } + { sleepFor }, + { maxDuration } ); return result; }, });These changes will improve the code quality and prevent potential issues with invalid input.
Line range hint
1-73: Summary: Implementation aligns with PR objectives, with minor improvements suggestedThe changes successfully implement the
maxDurationfeature for tasks as described in the PR objectives. The new tasks,maxDurationTaskandmaxDurationParentTask, provide the functionality to set and enforce maximum durations for task execution.Key points:
- The
maxDurationcan be set as intended in the task-specific context.- The implementation allows for overriding the
maxDurationwhen triggering a task.- The default values align with the PR objectives.
To further improve the implementation, please address the suggested changes in the previous comments, including:
- Verifying the necessity of the
usageimport.- Improving the error message in
maxDurationTask.- Removing the unused
ctxparameter and adding input validation inmaxDurationParentTask.Once these minor improvements are made, the implementation will be robust and ready for merging.
1-1: Verify the necessity of theusageimportThe
usageimport has been added from "@trigger.dev/sdk/v3", but it doesn't appear to be used in the visible code. Please ensure that this import is necessary for the implementation of the new features or remove it if it's not needed.To check if
usageis used elsewhere in the file or project, run:✅ Verification successful
Remove the unused
usageimportThe
usageimport from "@trigger.dev/sdk/v3" is not used in theexample.tsfile. Please remove it to clean up the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of the 'usage' import rg --type typescript 'usage\.' references/hello-world/src/trigger/Length of output: 103
Script:
#!/bin/bash # Search for usage of the 'usage' import in .ts and .tsx files rg 'usage\.' --glob '*.ts' --glob '*.tsx' references/hello-world/src/trigger/Length of output: 79
apps/webapp/app/v3/taskStatus.ts (2)
44-44: LGTM: Addition of "TIMED_OUT" to FINAL_RUN_STATUSES is correct.The addition of "TIMED_OUT" to FINAL_RUN_STATUSES is consistent with the PR objectives and correctly categorizes it as a final status for a task run.
To ensure this change is reflected in the rest of the codebase, please run the following script:
✅ Verification successful
Verification Successful: "TIMED_OUT" is properly used throughout the codebase.
No further action is required.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of "TIMED_OUT" status in the codebase # Test: Search for "TIMED_OUT" usage in TypeScript files rg --type typescript '"TIMED_OUT"' # Test: Check if there are any switch statements or if-else chains handling TaskRunStatus that might need updating ast-grep --lang typescript --pattern 'switch ($_) { $$$case "TIMED_OUT":$$$ }' ast-grep --lang typescript --pattern 'if ($_ === "TIMED_OUT") { $$$ }'Length of output: 225
Script:
#!/bin/bash # Description: Verify the usage of "TIMED_OUT" status in the codebase with corrected file type # Test: Search for "TIMED_OUT" usage in TypeScript files using the correct type rg --type ts '"TIMED_OUT"' # Test: Check if there are any switch statements or if-else chains handling TaskRunStatus that might need updating with correct lang ast-grep --lang typescript --pattern 'switch ($_) { $$$case "TIMED_OUT":$$$ }' ast-grep --lang typescript --pattern 'if ($_ === "TIMED_OUT") { $$$ }'Length of output: 3299
100-100: LGTM: Addition of "TIMED_OUT" to FAILED_RUN_STATUSES is correct.The addition of "TIMED_OUT" to FAILED_RUN_STATUSES is consistent with the PR objectives and correctly categorizes it as a failed status for a task run.
To ensure this change is reflected in the rest of the codebase, please run the following script:
✅ Verification successful
Please run the following script to verify the handling of the
"TIMED_OUT"status as a failed status in the codebase:
: Addition of "TIMED_OUT" to FAILED_RUN_STATUSES has been successfully verified.
The inclusion of "TIMED_OUT" in FAILED_RUN_STATUSES is correctly handled through the
isFailedRunStatusfunction across the codebase. No additional individual conditionals for "TIMED_OUT" were found, ensuring consistent treatment as a failed status.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of "TIMED_OUT" status as a failed status in the codebase # Test: Search for usage of isFailedRunStatus function rg --type typescript 'isFailedRunStatus' # Test: Check if there are any places where failed run statuses are handled individually ast-grep --lang typescript --pattern 'if ($_ === "FAILED" || $_ === "INTERRUPTED" || $_ === "COMPLETED_WITH_ERRORS" || $_ === "SYSTEM_FAILURE" || $_ === "CRASHED") { $$$ }'Length of output: 250
Script:
# #!/bin/bash # Description: Verify the handling of "TIMED_OUT" status as a failed status in the codebase # Test: Search for usage of isFailedRunStatus function in TypeScript files rg 'isFailedRunStatus' --glob '*.ts' --glob '*.tsx' # Test: Check if there are any places where failed run statuses are handled individually, including "TIMED_OUT" ast-grep --lang typescript --pattern ' if ($_ === "FAILED" || $_ === "INTERRUPTED" || $_ === "COMPLETED_WITH_ERRORS" || $_ === "SYSTEM_FAILURE" || $_ === "CRASHED" || $_ === "TIMED_OUT") { $$$ } 'Length of output: 608
packages/core/src/v3/schemas/api.ts (2)
447-448: LGTM! Well-documented addition to RunStatus enum.The addition of the
MAX_DURATION_EXCEEDEDstatus to theRunStatusenum is well-implemented and aligns perfectly with the PR objectives. The accompanying comment clearly explains the purpose of this new status, enhancing code readability and maintainability.
87-87: Verify integration of maxDuration feature in the codebaseThe changes in this file successfully implement the
maxDurationfeature as per the PR objectives. Both theTriggerTaskRequestBodyschema and theRunStatusenum have been updated appropriately.To ensure full integration of this feature, please run the following script to check for proper usage of
maxDurationand handling of theMAX_DURATION_EXCEEDEDstatus throughout the codebase:This will help ensure that the new feature is properly integrated and handled throughout the project.
Also applies to: 447-448
✅ Verification successful
maxDuration feature is properly integrated and utilized
The
maxDurationproperty and theMAX_DURATION_EXCEEDEDstatus are correctly implemented and used throughout the codebase as intended. No issues were found during the verification process.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of maxDuration and MAX_DURATION_EXCEEDED # Check for usage of maxDuration echo "Checking usage of maxDuration:" rg --type typescript 'maxDuration' # Check for handling of MAX_DURATION_EXCEEDED status echo "Checking handling of MAX_DURATION_EXCEEDED status:" rg --type typescript 'MAX_DURATION_EXCEEDED'Length of output: 331
Script:
#!/bin/bash # Description: Verify usage of maxDuration and MAX_DURATION_EXCEEDED in TypeScript files # Check for usage of maxDuration echo "Checking usage of maxDuration:" rg --glob "*.{ts,tsx}" 'maxDuration' # Check for handling of MAX_DURATION_EXCEEDED status echo "Checking handling of MAX_DURATION_EXCEEDED status:" rg --glob "*.{ts,tsx}" 'MAX_DURATION_EXCEEDED'Length of output: 7518
apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (2)
42-48: LGTM: Enhanced imports for task run status handlingThe additional imports (
descriptionForTaskRunStatus,filterableTaskRunStatuses,runStatusTitle) from theTaskRunStatusmodule align well with the PR objectives. These imports suggest improved functionality for handling and displaying task run statuses.
314-317: LGTM: Improved status display with tooltipThe addition of a
SimpleTooltipcomponent to wrap theTaskRunStatusCombois an excellent improvement. It provides users with a detailed description of each task run status on hover, enhancing the clarity and usability of the interface. This change aligns perfectly with the PR objectives of improving the display and interaction of task run statuses.apps/webapp/app/v3/services/completeAttempt.server.ts (1)
366-375: Ensure 'TIMED_OUT' status is properly handled throughout the codebaseThe new status
'TIMED_OUT'is introduced when the error code is'MAX_DURATION_EXCEEDED'. Please verify that this status is recognized and appropriately handled in all relevant parts of the codebase, such as state management, logging, UI components, and any business logic that depends on task run statuses.Run the following script to search for all usages of
'TIMED_OUT'status to confirm it's handled properly:✅ Verification successful
'TIMED_OUT' Status Properly Handled
All occurrences of the 'TIMED_OUT' status are appropriately managed across the codebase, including state management, UI components, and business logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'TIMED_OUT' status is handled across the codebase. # Test: Search for occurrences of 'TIMED_OUT' status. Expect: Find code handling this status. rg --fixed-strings 'TIMED_OUT' --type ts -A 5 -B 5Length of output: 35133
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/webapp/app/assets/icons/TimedOutIcon.tsx (1)
11-16: LGTM: Well-defined path element with a suggestion for improvementThe path element is well-defined with appropriate attributes. The use of "currentColor" for the fill allows for flexible theming, which is excellent.
Consider adding a
role="img"attribute to the SVG element and anaria-labeldescribing the icon for improved accessibility. For example:<svg className={className} width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Timed out icon" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- apps/webapp/app/assets/icons/TimedOutIcon.tsx (1 hunks)
- apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (7 hunks)
🔇 Additional comments (9)
apps/webapp/app/assets/icons/TimedOutIcon.tsx (3)
1-2: LGTM: Well-structured component declarationThe component declaration is well-structured, using TypeScript for prop typing and following React naming conventions. The optional
classNameprop allows for flexible styling, which is a good practice for reusable components.
3-10: LGTM: Properly configured SVG elementThe SVG element is well-configured with appropriate attributes. The use of the
classNameprop for custom styling and the consistent sizing (20x20) are good practices for a reusable icon component.
1-19: Great implementation of the TimedOutIcon componentThis component is well-implemented and aligns perfectly with the PR objectives of introducing a
maxDurationfeature for tasks. The icon effectively represents the concept of a "timed out" or "duration exceeded" state, which will be useful for visual feedback when a task exceeds its maximum allowed duration.The component is concise, reusable, and follows React best practices. The use of TypeScript for prop typing and the flexible styling options make it a valuable addition to the project's UI components.
apps/webapp/app/components/runs/v3/TaskRunStatus.tsx (6)
18-18: Import ofTimedOutIconis correctThe
TimedOutIconcomponent is correctly imported from"~/assets/icons/TimedOutIcon".
32-32: AddingTIMED_OUTtoallTaskRunStatusesThe
TIMED_OUTstatus has been appropriately added to theallTaskRunStatusesarray.
50-50: AddingTIMED_OUTtofilterableTaskRunStatusesThe
TIMED_OUTstatus has been appropriately added to thefilterableTaskRunStatusesarray.
148-149: HandlingTIMED_OUTinTaskRunStatusIconThe
TIMED_OUTstatus is correctly handled in theTaskRunStatusIconcomponent with theTimedOutIcon.
184-185: Assignment oftext-errorclass forTIMED_OUTstatusThe
TIMED_OUTstatus correctly returns"text-error"in therunStatusClassNameColorfunction, maintaining consistency with other error statuses.
222-223: Setting the title forTIMED_OUTstatusThe
TIMED_OUTstatus title is appropriately set to"Timed out"in therunStatusTitlefunction.
ericallam
force-pushed
the
feat/run-max-duration
branch
from
c8fbd28 to
474d6d3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
.changeset/tiny-forks-remember.md (1)
1-7: LGTM! Consider enhancing the description slightly.The changeset file correctly identifies the packages to be updated and uses appropriate version bumps. The description clearly states the purpose of the new feature.
Consider slightly expanding the description for more clarity:
-Adding maxDuration to tasks to allow timing out runs after they exceed a certain number of seconds +Adding maxDuration to tasks to allow timing out runs after they exceed a specified duration in seconds. This can be set globally, per task, or at runtime.This addition provides a bit more context about the flexibility of the new feature, aligning closely with the PR objectives.
packages/trigger-sdk/src/v3/timeout.ts (2)
3-3: LGTM: Constant definition is appropriate. Consider adding a comment.The
MAXIMUM_MAX_DURATIONconstant is correctly defined as the maximum value for a 32-bit signed integer. The use of underscores for digit grouping improves readability.Consider adding a brief comment explaining the significance of this value:
+// Maximum value for a 32-bit signed integer, used to represent "no timeout" const MAXIMUM_MAX_DURATION = 2_147_483_647;
5-8: LGTM: Exportedtimeoutobject is well-structured. Consider adding JSDoc comments.The exported
timeoutobject is correctly implemented withNonerepresenting "no timeout" andsignalproviding access to the imported API's signal. This structure aligns well with the PR objectives for configuring task durations.Consider adding JSDoc comments to improve clarity:
+/** + * Timeout configuration object + * @property {number} None - Represents no timeout (maximum possible duration) + * @property {Function} signal - Timeout signal from the core API + */ export const timeout = { None: MAXIMUM_MAX_DURATION, signal: timeoutApi.signal, };apps/webapp/app/v3/utils/maxDuration.ts (2)
1-2: LGTM! Consider exporting constants for reusability.The constants
MINIMUM_MAX_DURATIONandMAXIMUM_MAX_DURATIONare well-defined and serve their purpose. The minimum value of 5 prevents extremely short durations, and using the largest 32-bit signed integer as the maximum is a common practice.Consider exporting these constants if they might be useful in other parts of the codebase:
-const MINIMUM_MAX_DURATION = 5; -const MAXIMUM_MAX_DURATION = 2_147_483_647; // largest 32-bit signed integer +export const MINIMUM_MAX_DURATION = 5; +export const MAXIMUM_MAX_DURATION = 2_147_483_647; // largest 32-bit signed integer
1-22: Overall, good implementation of maxDuration utility functions.The
maxDuration.tsfile successfully implements utility functions for handling themaxDurationfeature as described in the PR objectives. The constants and functions provide a robust way to handle and validate maxDuration values, which will be crucial for the task execution time limit feature.The implementation aligns well with the PR's goal of introducing a
maxDurationfeature for tasks, allowing for global defaults, task-specific settings, and on-the-fly overrides. The utility functions in this file will likely be used to enforce these settings consistently across the application.As the
maxDurationfeature is implemented across the application, ensure that these utility functions are used consistently to maintain uniform behavior. Consider creating unit tests for these functions to verify their behavior, especially for edge cases and the specialMAXIMUM_MAX_DURATIONscenario.packages/trigger-sdk/src/v3/index.ts (1)
Line range hint
1-54: Summary: Timeout functionality exported, but further review needed.The addition of the timeout export is a good start for implementing the
maxDurationfeature. However, to fully meet the PR objectives, we should also review:
- The implementation in
timeout.jsto ensure it covers all the required functionality (global default, task-specific, and on-the-fly override).- Any changes to
trigger.config.tsfor setting the global default.- Updates to task definition interfaces or functions to allow setting task-specific
maxDuration.- Modifications to the
triggercommand to support on-the-flymaxDurationoverride.- Implementation of the error handling and message display when a task exceeds its
maxDuration.These areas are crucial to ensure the complete and correct implementation of the
maxDurationfeature as described in the PR objectives.apps/webapp/app/v3/services/triggerTask.server.ts (1)
377-379: LGTM! Consider adding explicit validation formaxDuration.The implementation of
maxDurationInSecondsaligns well with the PR objectives. It correctly handles the optional nature of themaxDurationsetting and uses theclampMaxDurationfunction to process the value.Consider adding explicit validation for the
maxDurationvalue before passing it toclampMaxDuration. This could help catch and handle invalid inputs early in the process. For example:if (body.options?.maxDuration !== undefined && typeof body.options.maxDuration !== 'number') { throw new ServiceValidationError('maxDuration must be a number.'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
- .changeset/tiny-forks-remember.md (1 hunks)
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx (4 hunks)
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts (1 hunks)
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts (4 hunks)
- apps/webapp/app/v3/marqs/devQueueConsumer.server.ts (2 hunks)
- apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts (3 hunks)
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts (2 hunks)
- apps/webapp/app/v3/services/triggerTask.server.ts (2 hunks)
- apps/webapp/app/v3/utils/maxDuration.ts (1 hunks)
- packages/core/src/v3/schemas/api.ts (2 hunks)
- packages/trigger-sdk/src/v3/index.ts (1 hunks)
- packages/trigger-sdk/src/v3/timeout.ts (1 hunks)
- references/hello-world/src/trigger/example.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
- apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts
- apps/webapp/app/presenters/v3/ApiRunListPresenter.server.ts
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts
- apps/webapp/app/v3/marqs/devQueueConsumer.server.ts
- apps/webapp/app/v3/marqs/sharedQueueConsumer.server.ts
- apps/webapp/app/v3/services/createBackgroundWorker.server.ts
- packages/core/src/v3/schemas/api.ts
🔇 Additional comments (10)
packages/trigger-sdk/src/v3/timeout.ts (2)
1-1: LGTM: Import statement is appropriate.The import of the
timeoutAPI from "@trigger.dev/core/v3" is correctly implemented. Renaming it totimeoutApiis a good practice to avoid naming conflicts with the exportedtimeoutobject.
1-8: Overall implementation looks good and aligns with PR objectives.The
timeout.tsfile successfully introduces themaxDurationfeature for tasks as described in the PR objectives. It provides a clean interface for setting "no timeout" and accessing the timeout signal. The implementation is concise, clear, and easy to understand.Some minor suggestions for improvement have been made in previous comments, but overall, this implementation effectively supports the new
maxDurationfeature for task execution.apps/webapp/app/v3/utils/maxDuration.ts (2)
4-6: LGTM! Efficient implementation of clamping logic.The
clampMaxDurationfunction effectively ensures that themaxDurationvalue is within the defined range. The use ofMath.minandMath.maxis an efficient and idiomatic way to implement clamping in JavaScript/TypeScript.
8-22: 🛠️ Refactor suggestionConsider using
clampMaxDurationand clarifyMAXIMUM_MAX_DURATIONcase.The
getMaxDurationfunction handles various scenarios well. However, there are two points to consider:
The function doesn't use the
clampMaxDurationfunction, which might be an oversight. Consider applyingclampMaxDurationto ensure the returned value is always within the allowed range.The special case for
MAXIMUM_MAX_DURATIONreturnsundefined. This behavior might be confusing as it differs from the function's return type annotation. Could you clarify the reasoning behind this?Consider refactoring the function to use
clampMaxDuration:export function getMaxDuration( maxDuration?: number | null, defaultMaxDuration?: number | null ): number | undefined { if (!maxDuration) { return defaultMaxDuration !== null ? clampMaxDuration(defaultMaxDuration) : undefined; } // Setting the maxDuration to MAXIMUM_MAX_DURATION means we don't want to use the default maxDuration if (maxDuration === MAXIMUM_MAX_DURATION) { return undefined; } return clampMaxDuration(maxDuration); }Could you explain the rationale behind returning
undefinedwhenmaxDuration === MAXIMUM_MAX_DURATION? This behavior seems to deviate from the function's purpose of providing a valid duration.packages/trigger-sdk/src/v3/index.ts (2)
Line range hint
1-54: File structure and organization look good.The new export for the timeout functionality fits well within the existing file structure. The index file maintains a clear organization, with logically grouped exports and a well-documented
configurefunction. This change doesn't introduce any apparent issues or inconsistencies.
11-11: New export added for timeout functionality.The addition of
export * from "./timeout.js";aligns with the PR objective of introducing amaxDurationfeature for tasks. This export makes the timeout-related functionality available to users of the SDK.To ensure this change is consistent with the PR objectives, let's verify the contents of the
timeout.jsfile:apps/webapp/app/v3/services/triggerTask.server.ts (1)
26-26: Verify the implementation ofclampMaxDurationThe
clampMaxDurationfunction is imported and used to process themaxDurationoption. To ensure the complete correctness of themaxDurationfeature, it's important to verify the implementation of this function.Please run the following script to check the implementation of
clampMaxDuration:This will help us confirm that
clampMaxDurationis implemented correctly and has appropriate test coverage.references/hello-world/src/trigger/example.ts (3)
60-70: ****The comment from the previous review is still applicable:
Remove unused
ctxparameter frommaxDurationParentTaskThe
ctxparameter is destructured in therunfunction but is not used within the function body. Removing it can improve clarity and avoid confusion.Apply this diff to remove the unused parameter:
export const maxDurationParentTask = task({ id: "max-duration-parent", - run: async (payload: { sleepFor?: number; maxDuration?: number }, { ctx, signal }) => { + run: async (payload: { sleepFor?: number; maxDuration?: number }, { signal }) => { const result = await maxDurationTask.triggerAndWait( { sleepFor: payload.sleepFor ?? 10 }, { maxDuration: timeout.None } ); return result; }, });
60-70: ****The comment from the previous review is still applicable:
Add input validation for
sleepForandmaxDurationparametersCurrently,
maxDurationParentTaskdoes not validate thesleepForandmaxDurationvalues. Consider adding checks to ensure these are positive numbers to prevent unexpected behavior.Apply this diff to add input validation:
export const maxDurationParentTask = task({ id: "max-duration-parent", run: async (payload: { sleepFor?: number; maxDuration?: number }, { signal }) => { + const sleepFor = payload.sleepFor ?? 10; + if (sleepFor <= 0) { + throw new Error("sleepFor must be a positive number"); + } const result = await maxDurationTask.triggerAndWait( - { sleepFor: payload.sleepFor ?? 10 }, + { sleepFor }, { maxDuration: timeout.None } ); return result; }, });
46-58: Verify themaxDurationconfiguration is applied correctly.The
maxDurationproperty is set to 5 seconds for themaxDurationTask. However, the task usessetTimeoutto pause execution based on thesleepForpayload value, which defaults to 10 seconds in the parent task.To verify the
maxDurationbehavior, run the following script:If the
maxDurationis not applied correctly, consider adjusting the value or thesleepFordefault in the parent task to align with the expected behavior.
commit 886429b Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:36:46 2024 +0100 Removed emails, @trigger.dev/database and @trigger.dev/otlp-importer from changesets config commit f65157a Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:31:27 2024 +0100 Lockfile with run-engine removed commit 3d67bb8 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:24:31 2024 +0100 Removed run-engine from the webapp package.json/tsconfig commit d30e971 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:06:04 2024 +0100 Dockerfile fix because the database package has been moved commit f2babbf Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 09:41:22 2024 -0700 Internal packages (testcontainers, redis-worker and zod-worker) (#1392) * Some notes on the new run engine * lockfile with setup for the run engine * Documenting where TaskRun is currently mutated, to try figure out the shape of the new system * Added notes about how triggering currently works * Details about when triggering happens * Lots of notes about waitpoints * Started scaffolding the RunEngine * Sketch of Prisma waitpoint schema while it’s fresh in my mind * Got Prisma working with testcontainers * Use beforeEach/afterEach * Simple Prisma and Redis test * Return Redis options instead of a client * Simplified things * A very simple FIFO pull-based queue to check the tests working properly * Use vitest extend * Separate redis, postgres and combined tests for faster testing * Some fixes and test improvements * Pass a logger into the queue * A queue processor that processes items from the given queue as fast as it can * Test for retrying an item that wasn’t processed * First draft of waitpoints in the Prisma schema * Remove the custom logger from the test * Added a completedAt to Waitpoint * Notes on the flow for an execution starting * Added redlock, moved some files around * Starting point for the TaskRunExecutionSnapshot table * Added relationships to TaskRunExecutionSnapshot * Change some tsconfig * Moved some things around * Added some packages * WIP on the RunQueue * Fix for some imports * Key producer with some tests * Removed the nv type from the keys… it’s not useful to do global queries * Passing unit tests for all the public key producer functions * Some basic tests passing for the RunQueue * Simple enqueue test working * Enqueue and dequeue for dev is working * Don’t log everything during the tests * Enqueuing/dequeuing from the shared queue is working * Tests for getting a shared queue * The key producer sharedQueue can now be named, to allow multiple separate queues * The key producer uses the name of the queue as the input * Extra info in the Prisma schema * Dequeuing a message gets the payload and sets the task concurrency all in one Lua script * Adding more keys so we can read the concurrency from the queue * Setting the concurrency with dequeue and enquque is working * Improved the tests and fixed some bugs * Acking is resetting the concurrencies * Check the key has been removed after acking * Nacking is working * Changed the package to CommonJS + Node10 so it works with Redlock * Moved the database, otel and emails packages to be in internal-packages * Moved some Prisma code to the database package * Started using the RunEngine for triggering * Progress on run engine triggering, first waitpoint code * Create a delay waitpoint * Moved ZodWorker to an internal package so it can be used in the run engine as well as the webapp * Web app now uses the zod worker package * Added parseNaturalLanguageDuration to core/apps * internal-packages/zod-worker in the lockfile * Pass in the master queue, remove old rebalance workers code * Add masterQueue to TaskRun * Fixed the tests * Moved waitpoint code into the run engine, also the zod worker * Completing waitpoints * An experiment to create a new test container with environment * More changes to triggering * Started testing triggering * Test for a run getting triggered and being enqueued * Removed dequeueMessageInEnv * Update dev queue tests to use the shared queue function * Schema changes for TaskRunExecutionSnapshot * First execution snapshot when the run is created. Dequeue run function added to the engine * Separate internal package for testcontainers so they can be used elsewhere * Remove the simple queue and testcontainers from the run-engine. They’re going to be separate * Fix for the wrong path to the Prisma schem,a * Added the testcontainers package to the run-engine * redis-worker package, just a copy of the simple queue for now * The queue now uses Lua to enqueue dequeue * The queue now has a catalog and an invisible period after dequeuing * Added a visibility timeout and acking, with tests * Added more Redis connection logging, deleted todos * Visibility timeouts are now defined on the catalog and can be overridden when enqueuing * Dequeue multiple items at once * Test for dequeuing multiple items * Export some types to be used elsewhere * Partial refactor of the processor * First stab at a worker with concurrency and NodeWorkers * Don’t have a default visibility timeout in the queue * Worker setup and processing items in a simple test * Process jobs in parallel with retrying * Get the attempt when dequeuing * Workers do exponential backoff * Moved todos * DLQ functionality * DLQ tests * Same cluster for all keys in the same queue * Added DLQ tests * Whitespace * Redis pubsub to redrive from the worker * Fixed database paths * Fix for path to zod-worker * Fixes for typecheck errors, mostly with TS versions and module resolution * Redlock required a patch * Moved the new DB migrations to the new database package folder * Remove the run-engine package * Remove the RunEngine prisma schema changes * Delete triggerTaskV2 * Remove zodworker test script (no tests) * Update test-containers readme * Generate the client first * Use a specific version of the prisma package * Generate the prisma client before running the unit tests commit fc60947 Author: Dan <8297864+D-K-P@users.noreply.github.com> Date: Tue Oct 8 14:36:03 2024 +0100 Supabase database webhook example upgrade (#1386) * Added overview for guides and examples section and split them all out * New supabase guide wip * Updated images and improved docs * Trimmed the supabase prereqs * Supabase guide wip * more updates * Replaced old database webhook guide * Created one intro page and removed snippets * Updated guide sidebar titles * Code updates * More improvements * Updates and added images * Compressed image * Updated guides descriptions and edge function basic * Removed bold * Updated redirects * Fixed broken links * Updated intro commit 07f82ea Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 13:28:54 2024 +0100 Release 3.0.11 commit 13ebfcc Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Oct 8 13:24:38 2024 +0100 chore: Update version for release (#1381) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 2a04d17 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:24:23 2024 +0100 Simplify showLogs expression commit 002ae4b Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:29 2024 +0100 Fix dotenv overrides for dev runs (#1388) * override dashboard dev env vars with local .env * add changeset * add simple task for testing env vars commit 047cb00 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:05 2024 +0100 Disable schedules for deleted orgs on next tick (#1383) * disable schedules for deleted orgs * add debug logs commit 2c014f7 Author: James Ritchie <james@trigger.dev> Date: Sun Oct 6 13:02:00 2024 -0700 Override log retention (#1385) * set full log retention as admin * If run.logsDeletedAt is set, don’t bother getting the trace commit a69e04f Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 14:18:58 2024 +0100 Include push output in logs for self-hosted deploys (#1382) * include push output in logs * changeset commit c5488df Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 13:12:47 2024 +0100 Fix CLI downgrade check (#1380) * fix downgrade detection * remove unused semver package from webapp * add changeset commit 1caec27 Author: Eric Allam <eric@trigger.dev> Date: Fri Oct 4 15:33:35 2024 -0700 docs: Max duration (#1379) * maxDuration docs * Update the init command to set the maxDuration and include a commented out maxDuration in the config file commit e14c954 Author: Eric Allam <eallam@icloud.com> Date: Fri Oct 4 15:02:05 2024 -0700 Release 3.0.10 commit 8e61f5d Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Oct 4 14:59:07 2024 -0700 chore: Update version for release (#1378) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 08db565 Author: Eric Allam <eallam@icloud.com> Date: Thu Oct 3 12:38:25 2024 -0700 improve the timed out description commit 6d08842 Author: Eric Allam <eric@trigger.dev> Date: Thu Oct 3 12:43:26 2024 -0700 feat: Add maxDuration to tasks (#1377) * WIP * Get max duration working on deployed runs * Actually set the timed out runs to status = TIMED_OUT * The client status for TIMED_OUT is now MAX_DURATION_EXCEEDED * New TimedOutIcon * Added new timedout icon * Add ability to opt-out of maxDuration with timeout.None * MAX_DURATION_EXCEEDED -> TIMED_OUT * changeset * Improved styling for the status tooltip content --------- Co-authored-by: James Ritchie <james@trigger.dev> commit 665ccf8 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Thu Oct 3 12:33:18 2024 +0100 Update github actions and self-hosting docs commit 1ff7b86 Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 18:26:36 2024 -0700 Add max queue depth limits (#1376) * Add runs to an env queue, as well as the actual queue * Add queue size limit guard on triggering tasks commit c531a9d Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 15:30:39 2024 -0700 fix: cleanup ttl expire run graphile jobs (#1373) * fix: remove ttl expire run graphile jobs when a run is started or completed * Update expireEnqueuedRun.server.ts commit 0bf500f Author: Matt Aitken <matt@mattaitken.com> Date: Wed Oct 2 15:30:16 2024 -0700 Prioritize finishing waited runs (#1375) * If a tree node is missing, estimate the size as zero * Task to test prioritizing finishing existing runs after triggerAndWaits * When requeuing a run with a checkpoint, put it in the queue with the parent run time so it’s correctly prioritized * The same change but if there’s no checkpoint
commit 886429b Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:36:46 2024 +0100 Removed emails, @trigger.dev/database and @trigger.dev/otlp-importer from changesets config commit f65157a Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:31:27 2024 +0100 Lockfile with run-engine removed commit 3d67bb8 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:24:31 2024 +0100 Removed run-engine from the webapp package.json/tsconfig commit d30e971 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:06:04 2024 +0100 Dockerfile fix because the database package has been moved commit f2babbf Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 09:41:22 2024 -0700 Internal packages (testcontainers, redis-worker and zod-worker) (#1392) * Some notes on the new run engine * lockfile with setup for the run engine * Documenting where TaskRun is currently mutated, to try figure out the shape of the new system * Added notes about how triggering currently works * Details about when triggering happens * Lots of notes about waitpoints * Started scaffolding the RunEngine * Sketch of Prisma waitpoint schema while it’s fresh in my mind * Got Prisma working with testcontainers * Use beforeEach/afterEach * Simple Prisma and Redis test * Return Redis options instead of a client * Simplified things * A very simple FIFO pull-based queue to check the tests working properly * Use vitest extend * Separate redis, postgres and combined tests for faster testing * Some fixes and test improvements * Pass a logger into the queue * A queue processor that processes items from the given queue as fast as it can * Test for retrying an item that wasn’t processed * First draft of waitpoints in the Prisma schema * Remove the custom logger from the test * Added a completedAt to Waitpoint * Notes on the flow for an execution starting * Added redlock, moved some files around * Starting point for the TaskRunExecutionSnapshot table * Added relationships to TaskRunExecutionSnapshot * Change some tsconfig * Moved some things around * Added some packages * WIP on the RunQueue * Fix for some imports * Key producer with some tests * Removed the nv type from the keys… it’s not useful to do global queries * Passing unit tests for all the public key producer functions * Some basic tests passing for the RunQueue * Simple enqueue test working * Enqueue and dequeue for dev is working * Don’t log everything during the tests * Enqueuing/dequeuing from the shared queue is working * Tests for getting a shared queue * The key producer sharedQueue can now be named, to allow multiple separate queues * The key producer uses the name of the queue as the input * Extra info in the Prisma schema * Dequeuing a message gets the payload and sets the task concurrency all in one Lua script * Adding more keys so we can read the concurrency from the queue * Setting the concurrency with dequeue and enquque is working * Improved the tests and fixed some bugs * Acking is resetting the concurrencies * Check the key has been removed after acking * Nacking is working * Changed the package to CommonJS + Node10 so it works with Redlock * Moved the database, otel and emails packages to be in internal-packages * Moved some Prisma code to the database package * Started using the RunEngine for triggering * Progress on run engine triggering, first waitpoint code * Create a delay waitpoint * Moved ZodWorker to an internal package so it can be used in the run engine as well as the webapp * Web app now uses the zod worker package * Added parseNaturalLanguageDuration to core/apps * internal-packages/zod-worker in the lockfile * Pass in the master queue, remove old rebalance workers code * Add masterQueue to TaskRun * Fixed the tests * Moved waitpoint code into the run engine, also the zod worker * Completing waitpoints * An experiment to create a new test container with environment * More changes to triggering * Started testing triggering * Test for a run getting triggered and being enqueued * Removed dequeueMessageInEnv * Update dev queue tests to use the shared queue function * Schema changes for TaskRunExecutionSnapshot * First execution snapshot when the run is created. Dequeue run function added to the engine * Separate internal package for testcontainers so they can be used elsewhere * Remove the simple queue and testcontainers from the run-engine. They’re going to be separate * Fix for the wrong path to the Prisma schem,a * Added the testcontainers package to the run-engine * redis-worker package, just a copy of the simple queue for now * The queue now uses Lua to enqueue dequeue * The queue now has a catalog and an invisible period after dequeuing * Added a visibility timeout and acking, with tests * Added more Redis connection logging, deleted todos * Visibility timeouts are now defined on the catalog and can be overridden when enqueuing * Dequeue multiple items at once * Test for dequeuing multiple items * Export some types to be used elsewhere * Partial refactor of the processor * First stab at a worker with concurrency and NodeWorkers * Don’t have a default visibility timeout in the queue * Worker setup and processing items in a simple test * Process jobs in parallel with retrying * Get the attempt when dequeuing * Workers do exponential backoff * Moved todos * DLQ functionality * DLQ tests * Same cluster for all keys in the same queue * Added DLQ tests * Whitespace * Redis pubsub to redrive from the worker * Fixed database paths * Fix for path to zod-worker * Fixes for typecheck errors, mostly with TS versions and module resolution * Redlock required a patch * Moved the new DB migrations to the new database package folder * Remove the run-engine package * Remove the RunEngine prisma schema changes * Delete triggerTaskV2 * Remove zodworker test script (no tests) * Update test-containers readme * Generate the client first * Use a specific version of the prisma package * Generate the prisma client before running the unit tests commit fc60947 Author: Dan <8297864+D-K-P@users.noreply.github.com> Date: Tue Oct 8 14:36:03 2024 +0100 Supabase database webhook example upgrade (#1386) * Added overview for guides and examples section and split them all out * New supabase guide wip * Updated images and improved docs * Trimmed the supabase prereqs * Supabase guide wip * more updates * Replaced old database webhook guide * Created one intro page and removed snippets * Updated guide sidebar titles * Code updates * More improvements * Updates and added images * Compressed image * Updated guides descriptions and edge function basic * Removed bold * Updated redirects * Fixed broken links * Updated intro commit 07f82ea Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 13:28:54 2024 +0100 Release 3.0.11 commit 13ebfcc Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Oct 8 13:24:38 2024 +0100 chore: Update version for release (#1381) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 2a04d17 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:24:23 2024 +0100 Simplify showLogs expression commit 002ae4b Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:29 2024 +0100 Fix dotenv overrides for dev runs (#1388) * override dashboard dev env vars with local .env * add changeset * add simple task for testing env vars commit 047cb00 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:05 2024 +0100 Disable schedules for deleted orgs on next tick (#1383) * disable schedules for deleted orgs * add debug logs commit 2c014f7 Author: James Ritchie <james@trigger.dev> Date: Sun Oct 6 13:02:00 2024 -0700 Override log retention (#1385) * set full log retention as admin * If run.logsDeletedAt is set, don’t bother getting the trace commit a69e04f Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 14:18:58 2024 +0100 Include push output in logs for self-hosted deploys (#1382) * include push output in logs * changeset commit c5488df Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 13:12:47 2024 +0100 Fix CLI downgrade check (#1380) * fix downgrade detection * remove unused semver package from webapp * add changeset commit 1caec27 Author: Eric Allam <eric@trigger.dev> Date: Fri Oct 4 15:33:35 2024 -0700 docs: Max duration (#1379) * maxDuration docs * Update the init command to set the maxDuration and include a commented out maxDuration in the config file commit e14c954 Author: Eric Allam <eallam@icloud.com> Date: Fri Oct 4 15:02:05 2024 -0700 Release 3.0.10 commit 8e61f5d Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Oct 4 14:59:07 2024 -0700 chore: Update version for release (#1378) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 08db565 Author: Eric Allam <eallam@icloud.com> Date: Thu Oct 3 12:38:25 2024 -0700 improve the timed out description commit 6d08842 Author: Eric Allam <eric@trigger.dev> Date: Thu Oct 3 12:43:26 2024 -0700 feat: Add maxDuration to tasks (#1377) * WIP * Get max duration working on deployed runs * Actually set the timed out runs to status = TIMED_OUT * The client status for TIMED_OUT is now MAX_DURATION_EXCEEDED * New TimedOutIcon * Added new timedout icon * Add ability to opt-out of maxDuration with timeout.None * MAX_DURATION_EXCEEDED -> TIMED_OUT * changeset * Improved styling for the status tooltip content --------- Co-authored-by: James Ritchie <james@trigger.dev> commit 665ccf8 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Thu Oct 3 12:33:18 2024 +0100 Update github actions and self-hosting docs commit 1ff7b86 Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 18:26:36 2024 -0700 Add max queue depth limits (#1376) * Add runs to an env queue, as well as the actual queue * Add queue size limit guard on triggering tasks commit c531a9d Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 15:30:39 2024 -0700 fix: cleanup ttl expire run graphile jobs (#1373) * fix: remove ttl expire run graphile jobs when a run is started or completed * Update expireEnqueuedRun.server.ts commit 0bf500f Author: Matt Aitken <matt@mattaitken.com> Date: Wed Oct 2 15:30:16 2024 -0700 Prioritize finishing waited runs (#1375) * If a tree node is missing, estimate the size as zero * Task to test prioritizing finishing existing runs after triggerAndWaits * When requeuing a run with a checkpoint, put it in the queue with the parent run time so it’s correctly prioritized * The same change but if there’s no checkpoint
* Checkbox component can have its label styles * Improved the dialog footer * Handle sending feedback to Slack using Plain * WIP making the modal conditional * Moved the Plain form action into the select plan file * removed comment * Show a confirmation diaglog if you’re downgrading from Pro to Hobby * Downgrading to Hobby works * Use redirectWithErrorMessage instead of throw error * The cancel form now submits the data correctly * Modals don’t trigger when you upgrade * Copy improvements * Added a tooltip to explain the link to the pricing page * Squashed commit of the following: commit 886429b Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:36:46 2024 +0100 Removed emails, @trigger.dev/database and @trigger.dev/otlp-importer from changesets config commit f65157a Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:31:27 2024 +0100 Lockfile with run-engine removed commit 3d67bb8 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:24:31 2024 +0100 Removed run-engine from the webapp package.json/tsconfig commit d30e971 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:06:04 2024 +0100 Dockerfile fix because the database package has been moved commit f2babbf Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 09:41:22 2024 -0700 Internal packages (testcontainers, redis-worker and zod-worker) (#1392) * Some notes on the new run engine * lockfile with setup for the run engine * Documenting where TaskRun is currently mutated, to try figure out the shape of the new system * Added notes about how triggering currently works * Details about when triggering happens * Lots of notes about waitpoints * Started scaffolding the RunEngine * Sketch of Prisma waitpoint schema while it’s fresh in my mind * Got Prisma working with testcontainers * Use beforeEach/afterEach * Simple Prisma and Redis test * Return Redis options instead of a client * Simplified things * A very simple FIFO pull-based queue to check the tests working properly * Use vitest extend * Separate redis, postgres and combined tests for faster testing * Some fixes and test improvements * Pass a logger into the queue * A queue processor that processes items from the given queue as fast as it can * Test for retrying an item that wasn’t processed * First draft of waitpoints in the Prisma schema * Remove the custom logger from the test * Added a completedAt to Waitpoint * Notes on the flow for an execution starting * Added redlock, moved some files around * Starting point for the TaskRunExecutionSnapshot table * Added relationships to TaskRunExecutionSnapshot * Change some tsconfig * Moved some things around * Added some packages * WIP on the RunQueue * Fix for some imports * Key producer with some tests * Removed the nv type from the keys… it’s not useful to do global queries * Passing unit tests for all the public key producer functions * Some basic tests passing for the RunQueue * Simple enqueue test working * Enqueue and dequeue for dev is working * Don’t log everything during the tests * Enqueuing/dequeuing from the shared queue is working * Tests for getting a shared queue * The key producer sharedQueue can now be named, to allow multiple separate queues * The key producer uses the name of the queue as the input * Extra info in the Prisma schema * Dequeuing a message gets the payload and sets the task concurrency all in one Lua script * Adding more keys so we can read the concurrency from the queue * Setting the concurrency with dequeue and enquque is working * Improved the tests and fixed some bugs * Acking is resetting the concurrencies * Check the key has been removed after acking * Nacking is working * Changed the package to CommonJS + Node10 so it works with Redlock * Moved the database, otel and emails packages to be in internal-packages * Moved some Prisma code to the database package * Started using the RunEngine for triggering * Progress on run engine triggering, first waitpoint code * Create a delay waitpoint * Moved ZodWorker to an internal package so it can be used in the run engine as well as the webapp * Web app now uses the zod worker package * Added parseNaturalLanguageDuration to core/apps * internal-packages/zod-worker in the lockfile * Pass in the master queue, remove old rebalance workers code * Add masterQueue to TaskRun * Fixed the tests * Moved waitpoint code into the run engine, also the zod worker * Completing waitpoints * An experiment to create a new test container with environment * More changes to triggering * Started testing triggering * Test for a run getting triggered and being enqueued * Removed dequeueMessageInEnv * Update dev queue tests to use the shared queue function * Schema changes for TaskRunExecutionSnapshot * First execution snapshot when the run is created. Dequeue run function added to the engine * Separate internal package for testcontainers so they can be used elsewhere * Remove the simple queue and testcontainers from the run-engine. They’re going to be separate * Fix for the wrong path to the Prisma schem,a * Added the testcontainers package to the run-engine * redis-worker package, just a copy of the simple queue for now * The queue now uses Lua to enqueue dequeue * The queue now has a catalog and an invisible period after dequeuing * Added a visibility timeout and acking, with tests * Added more Redis connection logging, deleted todos * Visibility timeouts are now defined on the catalog and can be overridden when enqueuing * Dequeue multiple items at once * Test for dequeuing multiple items * Export some types to be used elsewhere * Partial refactor of the processor * First stab at a worker with concurrency and NodeWorkers * Don’t have a default visibility timeout in the queue * Worker setup and processing items in a simple test * Process jobs in parallel with retrying * Get the attempt when dequeuing * Workers do exponential backoff * Moved todos * DLQ functionality * DLQ tests * Same cluster for all keys in the same queue * Added DLQ tests * Whitespace * Redis pubsub to redrive from the worker * Fixed database paths * Fix for path to zod-worker * Fixes for typecheck errors, mostly with TS versions and module resolution * Redlock required a patch * Moved the new DB migrations to the new database package folder * Remove the run-engine package * Remove the RunEngine prisma schema changes * Delete triggerTaskV2 * Remove zodworker test script (no tests) * Update test-containers readme * Generate the client first * Use a specific version of the prisma package * Generate the prisma client before running the unit tests commit fc60947 Author: Dan <8297864+D-K-P@users.noreply.github.com> Date: Tue Oct 8 14:36:03 2024 +0100 Supabase database webhook example upgrade (#1386) * Added overview for guides and examples section and split them all out * New supabase guide wip * Updated images and improved docs * Trimmed the supabase prereqs * Supabase guide wip * more updates * Replaced old database webhook guide * Created one intro page and removed snippets * Updated guide sidebar titles * Code updates * More improvements * Updates and added images * Compressed image * Updated guides descriptions and edge function basic * Removed bold * Updated redirects * Fixed broken links * Updated intro commit 07f82ea Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 13:28:54 2024 +0100 Release 3.0.11 commit 13ebfcc Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Oct 8 13:24:38 2024 +0100 chore: Update version for release (#1381) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 2a04d17 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:24:23 2024 +0100 Simplify showLogs expression commit 002ae4b Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:29 2024 +0100 Fix dotenv overrides for dev runs (#1388) * override dashboard dev env vars with local .env * add changeset * add simple task for testing env vars commit 047cb00 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:05 2024 +0100 Disable schedules for deleted orgs on next tick (#1383) * disable schedules for deleted orgs * add debug logs commit 2c014f7 Author: James Ritchie <james@trigger.dev> Date: Sun Oct 6 13:02:00 2024 -0700 Override log retention (#1385) * set full log retention as admin * If run.logsDeletedAt is set, don’t bother getting the trace commit a69e04f Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 14:18:58 2024 +0100 Include push output in logs for self-hosted deploys (#1382) * include push output in logs * changeset commit c5488df Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 13:12:47 2024 +0100 Fix CLI downgrade check (#1380) * fix downgrade detection * remove unused semver package from webapp * add changeset commit 1caec27 Author: Eric Allam <eric@trigger.dev> Date: Fri Oct 4 15:33:35 2024 -0700 docs: Max duration (#1379) * maxDuration docs * Update the init command to set the maxDuration and include a commented out maxDuration in the config file commit e14c954 Author: Eric Allam <eallam@icloud.com> Date: Fri Oct 4 15:02:05 2024 -0700 Release 3.0.10 commit 8e61f5d Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Oct 4 14:59:07 2024 -0700 chore: Update version for release (#1378) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 08db565 Author: Eric Allam <eallam@icloud.com> Date: Thu Oct 3 12:38:25 2024 -0700 improve the timed out description commit 6d08842 Author: Eric Allam <eric@trigger.dev> Date: Thu Oct 3 12:43:26 2024 -0700 feat: Add maxDuration to tasks (#1377) * WIP * Get max duration working on deployed runs * Actually set the timed out runs to status = TIMED_OUT * The client status for TIMED_OUT is now MAX_DURATION_EXCEEDED * New TimedOutIcon * Added new timedout icon * Add ability to opt-out of maxDuration with timeout.None * MAX_DURATION_EXCEEDED -> TIMED_OUT * changeset * Improved styling for the status tooltip content --------- Co-authored-by: James Ritchie <james@trigger.dev> commit 665ccf8 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Thu Oct 3 12:33:18 2024 +0100 Update github actions and self-hosting docs commit 1ff7b86 Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 18:26:36 2024 -0700 Add max queue depth limits (#1376) * Add runs to an env queue, as well as the actual queue * Add queue size limit guard on triggering tasks commit c531a9d Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 15:30:39 2024 -0700 fix: cleanup ttl expire run graphile jobs (#1373) * fix: remove ttl expire run graphile jobs when a run is started or completed * Update expireEnqueuedRun.server.ts commit 0bf500f Author: Matt Aitken <matt@mattaitken.com> Date: Wed Oct 2 15:30:16 2024 -0700 Prioritize finishing waited runs (#1375) * If a tree node is missing, estimate the size as zero * Task to test prioritizing finishing existing runs after triggerAndWaits * When requeuing a run with a checkpoint, put it in the queue with the parent run time so it’s correctly prioritized * The same change but if there’s no checkpoint * Squashed commit of the following: commit 886429b Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:36:46 2024 +0100 Removed emails, @trigger.dev/database and @trigger.dev/otlp-importer from changesets config commit f65157a Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:31:27 2024 +0100 Lockfile with run-engine removed commit 3d67bb8 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:24:31 2024 +0100 Removed run-engine from the webapp package.json/tsconfig commit d30e971 Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 18:06:04 2024 +0100 Dockerfile fix because the database package has been moved commit f2babbf Author: Matt Aitken <matt@mattaitken.com> Date: Tue Oct 8 09:41:22 2024 -0700 Internal packages (testcontainers, redis-worker and zod-worker) (#1392) * Some notes on the new run engine * lockfile with setup for the run engine * Documenting where TaskRun is currently mutated, to try figure out the shape of the new system * Added notes about how triggering currently works * Details about when triggering happens * Lots of notes about waitpoints * Started scaffolding the RunEngine * Sketch of Prisma waitpoint schema while it’s fresh in my mind * Got Prisma working with testcontainers * Use beforeEach/afterEach * Simple Prisma and Redis test * Return Redis options instead of a client * Simplified things * A very simple FIFO pull-based queue to check the tests working properly * Use vitest extend * Separate redis, postgres and combined tests for faster testing * Some fixes and test improvements * Pass a logger into the queue * A queue processor that processes items from the given queue as fast as it can * Test for retrying an item that wasn’t processed * First draft of waitpoints in the Prisma schema * Remove the custom logger from the test * Added a completedAt to Waitpoint * Notes on the flow for an execution starting * Added redlock, moved some files around * Starting point for the TaskRunExecutionSnapshot table * Added relationships to TaskRunExecutionSnapshot * Change some tsconfig * Moved some things around * Added some packages * WIP on the RunQueue * Fix for some imports * Key producer with some tests * Removed the nv type from the keys… it’s not useful to do global queries * Passing unit tests for all the public key producer functions * Some basic tests passing for the RunQueue * Simple enqueue test working * Enqueue and dequeue for dev is working * Don’t log everything during the tests * Enqueuing/dequeuing from the shared queue is working * Tests for getting a shared queue * The key producer sharedQueue can now be named, to allow multiple separate queues * The key producer uses the name of the queue as the input * Extra info in the Prisma schema * Dequeuing a message gets the payload and sets the task concurrency all in one Lua script * Adding more keys so we can read the concurrency from the queue * Setting the concurrency with dequeue and enquque is working * Improved the tests and fixed some bugs * Acking is resetting the concurrencies * Check the key has been removed after acking * Nacking is working * Changed the package to CommonJS + Node10 so it works with Redlock * Moved the database, otel and emails packages to be in internal-packages * Moved some Prisma code to the database package * Started using the RunEngine for triggering * Progress on run engine triggering, first waitpoint code * Create a delay waitpoint * Moved ZodWorker to an internal package so it can be used in the run engine as well as the webapp * Web app now uses the zod worker package * Added parseNaturalLanguageDuration to core/apps * internal-packages/zod-worker in the lockfile * Pass in the master queue, remove old rebalance workers code * Add masterQueue to TaskRun * Fixed the tests * Moved waitpoint code into the run engine, also the zod worker * Completing waitpoints * An experiment to create a new test container with environment * More changes to triggering * Started testing triggering * Test for a run getting triggered and being enqueued * Removed dequeueMessageInEnv * Update dev queue tests to use the shared queue function * Schema changes for TaskRunExecutionSnapshot * First execution snapshot when the run is created. Dequeue run function added to the engine * Separate internal package for testcontainers so they can be used elsewhere * Remove the simple queue and testcontainers from the run-engine. They’re going to be separate * Fix for the wrong path to the Prisma schem,a * Added the testcontainers package to the run-engine * redis-worker package, just a copy of the simple queue for now * The queue now uses Lua to enqueue dequeue * The queue now has a catalog and an invisible period after dequeuing * Added a visibility timeout and acking, with tests * Added more Redis connection logging, deleted todos * Visibility timeouts are now defined on the catalog and can be overridden when enqueuing * Dequeue multiple items at once * Test for dequeuing multiple items * Export some types to be used elsewhere * Partial refactor of the processor * First stab at a worker with concurrency and NodeWorkers * Don’t have a default visibility timeout in the queue * Worker setup and processing items in a simple test * Process jobs in parallel with retrying * Get the attempt when dequeuing * Workers do exponential backoff * Moved todos * DLQ functionality * DLQ tests * Same cluster for all keys in the same queue * Added DLQ tests * Whitespace * Redis pubsub to redrive from the worker * Fixed database paths * Fix for path to zod-worker * Fixes for typecheck errors, mostly with TS versions and module resolution * Redlock required a patch * Moved the new DB migrations to the new database package folder * Remove the run-engine package * Remove the RunEngine prisma schema changes * Delete triggerTaskV2 * Remove zodworker test script (no tests) * Update test-containers readme * Generate the client first * Use a specific version of the prisma package * Generate the prisma client before running the unit tests commit fc60947 Author: Dan <8297864+D-K-P@users.noreply.github.com> Date: Tue Oct 8 14:36:03 2024 +0100 Supabase database webhook example upgrade (#1386) * Added overview for guides and examples section and split them all out * New supabase guide wip * Updated images and improved docs * Trimmed the supabase prereqs * Supabase guide wip * more updates * Replaced old database webhook guide * Created one intro page and removed snippets * Updated guide sidebar titles * Code updates * More improvements * Updates and added images * Compressed image * Updated guides descriptions and edge function basic * Removed bold * Updated redirects * Fixed broken links * Updated intro commit 07f82ea Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 13:28:54 2024 +0100 Release 3.0.11 commit 13ebfcc Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Oct 8 13:24:38 2024 +0100 chore: Update version for release (#1381) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 2a04d17 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:24:23 2024 +0100 Simplify showLogs expression commit 002ae4b Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:29 2024 +0100 Fix dotenv overrides for dev runs (#1388) * override dashboard dev env vars with local .env * add changeset * add simple task for testing env vars commit 047cb00 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Tue Oct 8 09:22:05 2024 +0100 Disable schedules for deleted orgs on next tick (#1383) * disable schedules for deleted orgs * add debug logs commit 2c014f7 Author: James Ritchie <james@trigger.dev> Date: Sun Oct 6 13:02:00 2024 -0700 Override log retention (#1385) * set full log retention as admin * If run.logsDeletedAt is set, don’t bother getting the trace commit a69e04f Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 14:18:58 2024 +0100 Include push output in logs for self-hosted deploys (#1382) * include push output in logs * changeset commit c5488df Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Sat Oct 5 13:12:47 2024 +0100 Fix CLI downgrade check (#1380) * fix downgrade detection * remove unused semver package from webapp * add changeset commit 1caec27 Author: Eric Allam <eric@trigger.dev> Date: Fri Oct 4 15:33:35 2024 -0700 docs: Max duration (#1379) * maxDuration docs * Update the init command to set the maxDuration and include a commented out maxDuration in the config file commit e14c954 Author: Eric Allam <eallam@icloud.com> Date: Fri Oct 4 15:02:05 2024 -0700 Release 3.0.10 commit 8e61f5d Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri Oct 4 14:59:07 2024 -0700 chore: Update version for release (#1378) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> commit 08db565 Author: Eric Allam <eallam@icloud.com> Date: Thu Oct 3 12:38:25 2024 -0700 improve the timed out description commit 6d08842 Author: Eric Allam <eric@trigger.dev> Date: Thu Oct 3 12:43:26 2024 -0700 feat: Add maxDuration to tasks (#1377) * WIP * Get max duration working on deployed runs * Actually set the timed out runs to status = TIMED_OUT * The client status for TIMED_OUT is now MAX_DURATION_EXCEEDED * New TimedOutIcon * Added new timedout icon * Add ability to opt-out of maxDuration with timeout.None * MAX_DURATION_EXCEEDED -> TIMED_OUT * changeset * Improved styling for the status tooltip content --------- Co-authored-by: James Ritchie <james@trigger.dev> commit 665ccf8 Author: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Thu Oct 3 12:33:18 2024 +0100 Update github actions and self-hosting docs commit 1ff7b86 Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 18:26:36 2024 -0700 Add max queue depth limits (#1376) * Add runs to an env queue, as well as the actual queue * Add queue size limit guard on triggering tasks commit c531a9d Author: Eric Allam <eric@trigger.dev> Date: Wed Oct 2 15:30:39 2024 -0700 fix: cleanup ttl expire run graphile jobs (#1373) * fix: remove ttl expire run graphile jobs when a run is started or completed * Update expireEnqueuedRun.server.ts commit 0bf500f Author: Matt Aitken <matt@mattaitken.com> Date: Wed Oct 2 15:30:16 2024 -0700 Prioritize finishing waited runs (#1375) * If a tree node is missing, estimate the size as zero * Task to test prioritizing finishing existing runs after triggerAndWaits * When requeuing a run with a checkpoint, put it in the queue with the parent run time so it’s correctly prioritized * The same change but if there’s no checkpoint * Revert "Squashed commit of the following:" This reverts commit b837b5a. * Removed console logs * cleaned up conditionals * Unlock free plan state * Fixed subscribe button if you’re already github verified * Simplified the downgrade reasons logic * made periodEnd required --------- Co-authored-by: nicktrn <55853254+nicktrn@users.noreply.github.com>
You can now add a
maxDurationto your tasks that will abort runs that exceed the duration, in CPU time only (e.g. it doesn't count when you are runningwait.forortriggerAndWait, etc.). You can setmaxDurationin 3 places:trigger.config.tsto apply as a default to all taskstrigger.config.tsif there is one)trigger, which overrides the previous two.If a task run exceeds the maxDuration (only across a single attempt) then the run will fail with the following error:
Summary by CodeRabbit
Release Notes
New Features
TimedOutIconfor visual representation of the "TIMED_OUT" status.Improvements
SpanPresenterand task run services to include maximum duration information.Bug Fixes