-
-
Notifications
You must be signed in to change notification settings - Fork 730
Feat: two phase deployment, version pinning #1739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: ab5725f The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
WalkthroughThis pull request introduces updates for deployment management across both the web interface and CLI. Unused imports were removed, and minor textual tweaks were made in UI components. New endpoints and service logic were added to support deployment promotion, replacing legacy rollback functionality. CLI commands now include promote options and the skipPromotion flag. Additionally, utilities and SDK functions were extended with enhancements for version and task handling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/CLI
participant C as CLI Api Client
participant A as API Endpoint
participant S as ChangeCurrentDeploymentService
participant DB as Database
U->>C: invoke promoteDeployment(version)
C->>A: POST /api/deployments/{version}/promote
A->>S: Validate & call promotion service
S->>DB: Upsert current deployment state
DB-->>S: Confirmation of update
S-->>A: Return promotion result
A-->>C: JSON response with deployment details
C-->>U: Display success message
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (13)
packages/cli-v3/src/utilities/githubActions.ts (2)
3-27
: Add error handling for file operations.The GitHub Actions utility function works correctly for setting outputs and environment variables, but lacks error handling for the file operations. If the append operation fails, there's no feedback or error capture.
Consider adding try/catch blocks to handle potential file operation errors:
- appendFileSync(process.env.GITHUB_ENV, contents); + try { + appendFileSync(process.env.GITHUB_ENV, contents); + } catch (error) { + console.error(`Failed to set GitHub environment variables: ${error.message}`); + }And similarly for the GITHUB_OUTPUT section.
10-17
: Consider adding feedback for non-GitHub Actions environments.The function silently does nothing when run outside a GitHub Actions environment. Adding a debug log or return value indicating whether the operations succeeded would improve debugging.
+ const isGitHubActions = Boolean(process.env.GITHUB_ENV || process.env.GITHUB_OUTPUT); + // Set environment variables if (process.env.GITHUB_ENV) { // existing code } // Set outputs if (process.env.GITHUB_OUTPUT) { // existing code } + + return isGitHubActions;apps/webapp/app/v3/utils/deploymentVersions.ts (1)
3-24
: Add input validation to ensure robust version comparison.The function works correctly for well-formed version strings, but lacks validation for malformed inputs. Consider adding checks for:
- Missing period character
- Non-numeric date or version components
- Empty string inputs
export function compareDeploymentVersions(versionA: string, versionB: string) { + // Validate inputs + if (!versionA || !versionB || !versionA.includes(".") || !versionB.includes(".")) { + throw new Error("Invalid version format. Expected format: YYYYMMDD.N"); + } + const [dateA, numberA] = versionA.split("."); const [dateB, numberB] = versionB.split("."); + + // Ensure numeric components + if (!/^\d+$/.test(dateA) || !/^\d+$/.test(dateB) || !/^\d+$/.test(numberA) || !/^\d+$/.test(numberB)) { + throw new Error("Version components must be numeric. Expected format: YYYYMMDD.N"); + } if (dateA < dateB) { return -1; } if (dateA > dateB) { return 1; } if (numberA < numberB) { return -1; } if (numberA > numberB) { return 1; } return 0; }apps/webapp/app/v3/services/finalizeDeployment.server.ts (1)
74-78
: LGTM! Clear implementation of optional promotion.The conditional promotion logic effectively implements the two-phase deployment approach, allowing deployments to be finalized without immediate promotion.
I recommend adding a comment explaining that promotion happens by default unless explicitly skipped, as this is a key behavior:
+// By default, we promote the deployment to be the current one unless skipPromotion is true if (typeof body.skipPromotion === "undefined" || !body.skipPromotion) { const promotionService = new ChangeCurrentDeploymentService(); await promotionService.call(finalizedDeployment, "promote"); }
apps/webapp/app/components/runs/v3/RollbackDeploymentDialog.tsx (2)
30-30
: Text change for consistency in deployment terminology.The header has been updated from "Roll back" to "Rollback" for consistency with terminology used elsewhere.
53-54
: Button text updated for terminology consistency.The button text has been updated to match the more consistent "Rollback" terminology.
apps/webapp/app/routes/api.v1.deployments.$deploymentVersion.promote.ts (1)
13-67
: Robust promotion action
• Checks the request method, validating it is POST.
• Authenticates request viaauthenticateApiRequest
.
• Verifies the target deployment exists.
• InvokesChangeCurrentDeploymentService
to handle promotion logic.
• Returns appropriate JSON responses and HTTP status codes.This flow looks solid. Consider concurrency controls if multiple promotions can happen simultaneously, but overall logic is good.
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts (1)
9-89
: Promotion/rollback logic
• Validates the deployment’s worker association and status.
• Compares versions to ensure correct forward/backward logic.
• Upserts a “current deployment” record and enqueues waiting tasks.Code is organized and errors are well-defined. You might consider DB transactions or locks if multiple version changes are triggered concurrently, but otherwise this is well-structured.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments/route.tsx (3)
45-48
: Importing both PromoteDeploymentDialog and RollbackDeploymentDialog from the same file may be confusing.
Although combining related dialogs in a single module can be convenient, consider renaming the file or splitting it to avoid confusion and keep each dialog’s purpose clearly distinguished.
327-337
: Optional param alignment forcurrentDeployment
.
The type signature accommodatescurrentDeployment
as an optional. Ensure all external references and default checks are in place to handle when it’s not provided.
383-402
: Promote menu item integrates well with rollback.
The new promote option parallels rollback’s structure, preserving user flow consistency. Good job on reusing the UI pattern.Consider user confirmation messaging (e.g., “Are you sure?”) to distinguish promote from rollback further and reduce accidental clicks.
packages/cli-v3/src/commands/promote.ts (1)
57-110
: Promotion workflow is straightforward and consistent.
- The check for a missing version is clear.
- The
intro
prompt is user-friendly.- Authorization & config resolution logic mirror other CLI commands.
- The final message confirms successful promotion.
These lines follow best practices for a CLI flow.If more advanced validations of the given version are needed, consider hooking into the CLI flow or an additional check for a valid “YYYYMMDD.x” format.
packages/trigger-sdk/src/v3/shared.ts (1)
27-1272
: Consider documenting version locking behavior for developers.The version locking functionality is well-implemented, but there's no documentation explaining how it works or when to use it. Consider adding JSDoc comments to explain this feature, especially since it's part of the new two-phase deployment system.
-export async function batchTriggerById<TTask extends AnyTask>( +/** + * Triggers multiple tasks by their identifiers with different payloads. + * + * @param items - Array of task items to trigger + * @param options - Optional batch trigger options + * @param requestOptions - Optional API request options + * @returns A promise resolving to a batch run handle + * + * @remarks + * Tasks can be locked to a specific version by using the `version` option or through + * the `TRIGGER_VERSION` environment variable. This is useful for multi-phase deployments + * to ensure consistent behavior during rollouts. + */ +export async function batchTriggerById<TTask extends AnyTask>(🧰 Tools
🪛 Biome (1.9.4)
[error] 135-135: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 151-151: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 167-167: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 190-190: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 280-280: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 297-297: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 313-313: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 336-336: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/webapp/app/components/primitives/Tabs.tsx
(1 hunks)apps/webapp/app/components/runs/v3/RollbackDeploymentDialog.tsx
(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments/route.tsx
(8 hunks)apps/webapp/app/routes/api.v1.deployments.$deploymentVersion.promote.ts
(1 hunks)apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts
(1 hunks)apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.rollback.ts
(2 hunks)apps/webapp/app/v3/authenticatedSocketConnection.server.ts
(1 hunks)apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
(1 hunks)apps/webapp/app/v3/services/finalizeDeployment.server.ts
(2 hunks)apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
(1 hunks)apps/webapp/app/v3/services/rollbackDeployment.server.ts
(0 hunks)apps/webapp/app/v3/utils/deploymentVersions.ts
(1 hunks)packages/cli-v3/src/apiClient.ts
(2 hunks)packages/cli-v3/src/cli/index.ts
(2 hunks)packages/cli-v3/src/commands/deploy.ts
(6 hunks)packages/cli-v3/src/commands/promote.ts
(1 hunks)packages/cli-v3/src/utilities/githubActions.ts
(1 hunks)packages/core/src/v3/index.ts
(1 hunks)packages/core/src/v3/schemas/api.ts
(2 hunks)packages/core/src/v3/types/tasks.ts
(1 hunks)packages/react-hooks/src/hooks/useTaskTrigger.ts
(1 hunks)packages/trigger-sdk/src/v3/shared.ts
(5 hunks)
💤 Files with no reviewable changes (1)
- apps/webapp/app/v3/services/rollbackDeployment.server.ts
✅ Files skipped from review due to trivial changes (1)
- apps/webapp/app/components/primitives/Tabs.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
🔇 Additional comments (43)
packages/core/src/v3/index.ts (1)
24-24
: Clean addition of the getEnv utility export.This addition correctly follows the established export pattern in the file and makes the environment utilities accessible throughout the application.
packages/cli-v3/src/cli/index.ts (1)
13-13
: Good integration of the new promote command.The import and configuration of the promote command follows the established pattern in the CLI structure, enhancing deployment management capabilities as part of the two-phase deployment feature.
Also applies to: 26-26
packages/react-hooks/src/hooks/useTaskTrigger.ts (1)
88-88
:✅ Verification successful
Confirm parameter naming consistency.
The code maps
options?.version
tolockToVersion
in the API call. While functionally correct, this naming difference might be confusing for developers. Consider using consistent naming across the codebase for version pinning functionality.
🏁 Script executed:
#!/bin/bash # Check for consistency in naming between version and lockToVersion in other files rg -A 2 "lockToVersion" --type ts rg -A 2 "version.*in task triggering context" --type tsLength of output: 3626
Parameter Naming Consistency Confirmed
After verifying the usage across the codebase, it’s clear that the mapping of
options?.version
tolockToVersion
is applied consistently. The API schema (e.g., inpackages/core/src/v3/schemas/api.ts
) and its corresponding usage in both trigger SDK and react-hooks adhere to this naming pattern. This indicates an intentional design decision rather than an inconsistency. No changes are necessary unless you decide to rework the convention across both the options and API parameters.apps/webapp/app/v3/services/finalizeDeployment.server.ts (1)
9-9
: LGTM!Clean import of the new
ChangeCurrentDeploymentService
.apps/webapp/app/v3/authenticatedSocketConnection.server.ts (1)
46-48
: LGTM! Method syntax change.The change from arrow function to regular method syntax doesn't alter functionality and is likely for consistency with the codebase style.
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts (1)
138-138
: LGTM! Proper parameter propagation.Correctly passes the
skipPromotion
flag from the V2 service to the original finalize service, ensuring consistent two-phase deployment behavior across both implementation paths.packages/core/src/v3/types/tasks.ts (2)
774-789
: Well-documented version support added to TriggerOptions.The addition of the
version
property to TriggerOptions provides an important capability for targeting specific deployment versions. The documentation clearly states this is only applicable for thetrigger
method and not fortriggerAndWait
.
792-795
: Properly updated TriggerAndWaitOptions to omit version property.The TriggerAndWaitOptions type has been correctly updated to omit the newly added
version
property alongside existing omitted properties. This maintains consistency with the documented limitation that version selection is only available for thetrigger
method.packages/cli-v3/src/apiClient.ts (2)
23-23
: Imported PromoteDeploymentResponseBody schema.The import was correctly added to support the new promoteDeployment method.
319-335
:✅ Verification successful
Well-implemented promoteDeployment method.
The new method follows the consistent pattern used throughout the class, with appropriate error handling for missing access tokens and proper use of the wrapZodFetch helper with the correct schema.
🏁 Script executed:
#!/bin/bash # Check if the promoteDeployment endpoint is properly used elsewhere in the codebase rg -A 3 "promoteDeployment\(" --type tsLength of output: 619
PromoteDeployment method confirmed: Implementation adheres to existing patterns.
The method throws an error if the access token is missing, correctly uses thewrapZodFetch
helper with the proper schema, and integrates neatly into the promote command as verified inpackages/cli-v3/src/commands/promote.ts
.
- Verified the method’s usage in both
packages/cli-v3/src/apiClient.ts
andpackages/cli-v3/src/commands/promote.ts
.apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.rollback.ts (2)
8-8
: Service import updated to use consolidated deployment service.The import change reflects the architectural shift from a dedicated RollbackDeploymentService to a more versatile ChangeCurrentDeploymentService that likely handles both rollback and promotion.
68-69
:❓ Verification inconclusive
Rollback implementation updated to use consolidated service.
The implementation now uses the ChangeCurrentDeploymentService with "rollback" as a parameter, maintaining the original functionality while leveraging the consolidated service architecture.
🏁 Script executed:
#!/bin/bash # Check if there are other instances of ChangeCurrentDeploymentService being used with different operations rg "new ChangeCurrentDeploymentService\(\).call\(" --type tsLength of output: 62
Rollback Update: Consolidated Service Usage Needs Manual Verification
The rollback logic now correctly instantiates and calls the consolidated
ChangeCurrentDeploymentService
with the"rollback"
parameter, maintaining the intended functionality. However, our automated search for other instances of this service with different operation parameters returned no output. Please manually verify that there are no other usages ofChangeCurrentDeploymentService
with alternative parameters elsewhere in the codebase.
- File to check:
apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.rollback.ts
(Lines 68-69)apps/webapp/app/components/runs/v3/RollbackDeploymentDialog.tsx (1)
61-101
: Added PromoteDeploymentDialog component for two-phase deployment support.A new dialog component has been added to support the promotion functionality, mirroring the structure of the rollback dialog but with appropriate text and actions for promotion. This component completes the UI for the two-phase deployment feature.
The descriptions are clear and help users understand the consequences of promotion: "This deployment will become the default for all future runs not explicitly tied to a specific deployment."
packages/cli-v3/src/commands/deploy.ts (6)
36-36
: New import looks good
This import introduces a helper function for GitHub Actions outputs and environment variables. No issues observed.
53-53
: Boolean default is appropriate
DefiningskipPromotion
with a default offalse
seems correct, since promotion is usually the desired behavior.
92-95
: CLI option name is clear
The--skip-promotion
flag name aligns with theskipPromotion
option. No concerns here—it's consistent and descriptive.
166-166
: Clear CLI feedback
Appending “(without promotion)” to the deployment intro message is helpful for user feedback. All good.
453-453
: Passing the option to finalizeDeployment
This ensures the newskipPromotion
option is respected during finalization. Verify downstream logic matches expectations.
485-506
: Appropriate GitHub Actions outputs
Storing these environment variables and outputs is useful for workflow steps. TheneedsPromotion
field correctly reflectsskipPromotion
. No security concerns noted.apps/webapp/app/routes/api.v1.deployments.$deploymentVersion.promote.ts (2)
1-7
: Imports and setup
The imported modules (@remix-run/server-runtime
,zod
, etc.) are relevant for request handling, DB access, and schema validation. Good use of consistent patterns.
9-11
: Parameter schema
Straightforward Zod validation fordeploymentVersion
. Any mismatched or missing param is handled with a 400 error.packages/core/src/v3/schemas/api.ts (2)
226-226
: Optional boolean aligns with CLI usage
MakingskipPromotion
optional allows flexibility. Ensure the server properly defaults to false if absent to match CLI’s default.
282-289
: Clear promotion response schema
Defines a minimal but sufficient shape (id
,version
,shortCode
) for a PromoteDeployment response. Good job.apps/webapp/app/v3/services/changeCurrentDeployment.server.ts (1)
1-7
: Imports and direction type
The new direction type (promote
orrollback
) clearly distinguishes actions. Import references appear valid.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.v3.$projectParam.deployments/route.tsx (6)
4-4
: Icon import looks good.
ImportingArrowUturnRightIcon
aligns with the new promote functionality and the existing icon usage pattern.
65-66
: Ensure consistent usage ofcompareDeploymentVersions
.
This import is likely used for verifying rollback versus promote conditions. Double-check that it’s correctly applied in all relevant conditions.
114-114
: Verifying uniqueness ofcurrentDeployment
.
This line assumes only one current deployment. Confirm thatdeployments.find(...)
returning the first match is sufficient or add checks to handle multiple flagged deployments, if that’s a possible scenario.
244-244
: Passing the newly introducedcurrentDeployment
prop.
The implementation looks good. Just ensure thatcurrentDeployment
isundefined
-safe for the child component.
341-349
: Condition logic is clear but consider renaming for clarity.
canBeMadeCurrent
is well-defined.canBeRolledBack
heavily depends oncompareDeploymentVersions
.canBePromoted
is an elegant extension.
If more conditions are introduced in the future, keep an eye on overall readability.
363-382
: Rollback menu item logic is consistent.
These lines maintain alignment with existing UI patterns and logic. TheDialogTrigger
approach is consistent with the app’s pattern for user-confirming actions.apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts (6)
1-8
: Import statements and base setup look good.
This file thoroughly covers standard dependencies (zod
,prisma
, custom redirect utils). No immediate issues found.
14-17
: Param schema is straightforward.
This is a good approach to validating URL parameters.
19-29
: Action function signature is correct, but watch naming.
The function references “rollbackSchema” but is implementing promotion logic. Otherwise, form data parsing is correct.
31-64
: Safely fetching project & deployment.
The query filters look appropriate for verifying user membership and existence. The usage ofredirectWithErrorMessage
is consistent with the rest of the app.
68-72
: Success message is user-friendly.
The confirmation clearly states the promoted version. This fosters good UX.
74-88
: Error handling remains robust.
Capturing both standard and non-Error objects is prudent. Logging the stack aids debugging.packages/cli-v3/src/commands/promote.ts (2)
1-49
: CLI command definition is solid.
- The new
promote
command includes helpful usage instructions and appropriate options (-c
,-e
, etc.).PromoteCommandOptions
extends sharedCommonCommandOptions
, ensuring consistency across commands.
Consider adding examples to the help text or additional validations (e.g., version format).
51-55
:promoteCommand
function is well-structured.
It delegates the validation toPromoteCommandOptions
and wraps the action neatly. This pattern keeps the code organized.packages/trigger-sdk/src/v3/shared.ts (5)
27-27
: Added new import for version locking functionality.The
getEnvVar
import is now used to retrieve theTRIGGER_VERSION
environment variable for task version locking.
618-618
: Added version locking capability for batch tasks.This change allows tasks to be locked to specific versions using either a version specified in options or falling back to the
TRIGGER_VERSION
environment variable. This supports the two-phase deployment feature by ensuring tasks run with the intended version.
955-955
: Consistent version locking implementation across batch functions.The version locking pattern is consistently implemented here, maintaining the same logic as other trigger functions.
1211-1211
: Added version locking for individual task triggers.This completes the implementation of version locking across all trigger functions, ensuring consistent behavior.
1272-1272
: Completed version locking implementation for all internal trigger functions.The version locking feature is now consistently implemented across all relevant functions in the codebase.
apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts
Outdated
Show resolved
Hide resolved
apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts (3)
14-17
: Consider exporting the ParamSchema for reusability.The
promoteSchema
is exported butParamSchema
is not. If this schema might be reused in related routes or tests, consider exporting it for consistency.-const ParamSchema = z.object({ +export const ParamSchema = z.object({ projectId: z.string(), deploymentShortCode: z.string(), });
73-88
: Consider adding specific HTTP status codes for error responses.The error handling is comprehensive, but the JSON responses don't include HTTP status codes. Adding appropriate status codes (e.g., 404 for not found, 403 for forbidden) would improve API clarity.
- return json(submission); + return json(submission, { status: 400 });- return json(submission); + return json(submission, { status: 500 });
30-63
: Consider enhancing error messages with more context.The current error messages ("Project not found", "Deployment not found") are generic. Adding more context, such as the project ID or deployment short code, could help with troubleshooting.
- return redirectWithErrorMessage(submission.value.redirectUrl, request, "Project not found"); + return redirectWithErrorMessage(submission.value.redirectUrl, request, `Project with ID ${projectId} not found`);- "Deployment not found" + `Deployment with short code ${deploymentShortCode} not found for project ${projectId}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/cold-coins-burn.md
(1 hunks)apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
.changeset/cold-coins-burn.md (2)
1-5
: Header and Patch Version Declarations
The header uses a clear YAML-style front matter format with the appropriate---
delimiters. The package declarations for"@trigger.dev/react-hooks"
,"@trigger.dev/sdk"
, and"trigger.dev"
are concisely marked for a patch update. This aligns well with the version pinning objective described in the PR.
7-8
: Descriptive Changeset Message
The description succinctly communicates the introduction of two-phase deployments and task version pinning. This message is clear and effectively summarizes the purpose of the changes, consistent with the PR objectives.apps/webapp/app/routes/resources.$projectId.deployments.$deploymentShortCode.promote.ts (3)
10-12
: Looks good - schema naming is appropriately aligned with functionality.The
promoteSchema
name correctly reflects the promotion functionality in this file, which is an improvement over previous versions that usedrollbackSchema
for the same purpose.
65-66
: Service naming aligns well with its purpose.The variable
promoteService
correctly reflects the promotion functionality, which resolves the previous cognitive dissonance issue where a rollback service was used for promotion operations.
1-90
:❓ Verification inconclusive
Verify transaction support for multi-step database operations.
The promotion process might involve multiple database changes, but there's no explicit transaction handling. Verify that
ChangeCurrentDeploymentService.call()
handles transactions internally or consider wrapping the operation in a transaction if needed.
🏁 Script executed:
#!/bin/bash # Search for transaction handling in the ChangeCurrentDeploymentService implementation rg -A 10 "class ChangeCurrentDeploymentService" "apps/webapp/app/v3/services" # Look for transaction usage in the service's call method rg -A 15 "call\(.*promote" "apps/webapp/app/v3/services"Length of output: 2759
Action Required: Verify Transaction Handling for Promote Operation
The promotion process involves multiple database updates, and our investigation did not reveal explicit transaction wrapping in the
ChangeCurrentDeploymentService.call()
implementation. Please verify that either:
- The service manages transactions internally (perhaps via BaseService or another delegated pattern), or
- The route should wrap the service call in a transaction to ensure atomic multi-step operations.
Steps to address:
- Confirm within
apps/webapp/app/v3/services/changeCurrentDeployment.server.ts
whether transaction logic (e.g., usingprisma.$transaction
) is invoked.- If not, consider wrapping the
promote
operation in a transaction for data consistency.- Review similar promotion paths (e.g., in
apps/webapp/app/v3/services/finalizeDeployment.server.ts
) for consistent transaction management.
Adds the ability to deploy to trigger.dev without immediately promoting that new deployment to the current version, using
--skip-promotion
:You can also now pass a version when triggering a task from your backend:
If you set the
TRIGGER_VERSION
env var in your calling code, that will automatically be used when triggering tasks.