-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(webapp): allow version downgrades via promote API #3214
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| area: webapp | ||
| type: feature | ||
| --- | ||
|
|
||
| Add allowRollbacks query param to the promote deployment API to enable version downgrades |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,11 @@ import { CURRENT_DEPLOYMENT_LABEL } from "@trigger.dev/core/v3/isomorphic"; | |
| export type ChangeCurrentDeploymentDirection = "promote" | "rollback"; | ||
|
|
||
| export class ChangeCurrentDeploymentService extends BaseService { | ||
| public async call(deployment: WorkerDeployment, direction: ChangeCurrentDeploymentDirection) { | ||
| public async call( | ||
| deployment: WorkerDeployment, | ||
| direction: ChangeCurrentDeploymentDirection, | ||
| disableVersionCheck?: boolean | ||
| ) { | ||
|
Comment on lines
+10
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Limit version-check bypass to promotions only. Line 49 currently disables both Suggested fix- if (!disableVersionCheck) {
- switch (direction) {
- case "promote": {
- if (
- compareDeploymentVersions(
- currentPromotion.deployment.version,
- deployment.version
- ) >= 0
- ) {
- throw new ServiceValidationError(
- "Cannot promote a deployment that is older than the current deployment."
- );
- }
- break;
- }
- case "rollback": {
- if (
- compareDeploymentVersions(
- currentPromotion.deployment.version,
- deployment.version
- ) <= 0
- ) {
- throw new ServiceValidationError(
- "Cannot rollback to a deployment that is newer than the current deployment."
- );
- }
- break;
- }
- }
- }
+ switch (direction) {
+ case "promote": {
+ if (
+ !disableVersionCheck &&
+ compareDeploymentVersions(
+ currentPromotion.deployment.version,
+ deployment.version
+ ) >= 0
+ ) {
+ throw new ServiceValidationError(
+ "Cannot promote a deployment that is older than the current deployment."
+ );
+ }
+ break;
+ }
+ case "rollback": {
+ if (
+ compareDeploymentVersions(
+ currentPromotion.deployment.version,
+ deployment.version
+ ) <= 0
+ ) {
+ throw new ServiceValidationError(
+ "Cannot rollback to a deployment that is newer than the current deployment."
+ );
+ }
+ break;
+ }
+ }Also applies to: 49-76 🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this is okay, given how isolated the change is. But happy to make this change too if reviewer feel strongly.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR description image shows a deployment table with two entries —
That said, I agree it's not a blocker given the current isolation. Happy to leave it as-is if the reviewer is comfortable — just flagging it as a low-cost guard for future maintainability. 🧠 Learnings used |
||
| if (!deployment.workerId) { | ||
| throw new ServiceValidationError( | ||
| direction === "promote" | ||
|
|
@@ -42,26 +46,34 @@ export class ChangeCurrentDeploymentService extends BaseService { | |
| } | ||
|
|
||
| // if there is a current promotion, we have to validate we are moving in the right direction based on the deployment versions | ||
| switch (direction) { | ||
| case "promote": { | ||
| if ( | ||
| compareDeploymentVersions(currentPromotion.deployment.version, deployment.version) >= 0 | ||
| ) { | ||
| throw new ServiceValidationError( | ||
| "Cannot promote a deployment that is older than the current deployment." | ||
| ); | ||
| if (!disableVersionCheck) { | ||
| switch (direction) { | ||
| case "promote": { | ||
| if ( | ||
| compareDeploymentVersions( | ||
| currentPromotion.deployment.version, | ||
| deployment.version | ||
| ) >= 0 | ||
| ) { | ||
| throw new ServiceValidationError( | ||
| "Cannot promote a deployment that is older than the current deployment." | ||
| ); | ||
| } | ||
| break; | ||
| } | ||
| break; | ||
| } | ||
| case "rollback": { | ||
| if ( | ||
| compareDeploymentVersions(currentPromotion.deployment.version, deployment.version) <= 0 | ||
| ) { | ||
| throw new ServiceValidationError( | ||
| "Cannot rollback to a deployment that is newer than the current deployment." | ||
| ); | ||
| case "rollback": { | ||
| if ( | ||
| compareDeploymentVersions( | ||
| currentPromotion.deployment.version, | ||
| deployment.version | ||
| ) <= 0 | ||
| ) { | ||
| throw new ServiceValidationError( | ||
| "Cannot rollback to a deployment that is newer than the current deployment." | ||
| ); | ||
| } | ||
| break; | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.