Skip to content

Revert "[RayService] Rollback Support for Incremental Upgrades"#4604

Closed
Future-Outlier wants to merge 2 commits intomasterfrom
revert-4109-rollback-support
Closed

Revert "[RayService] Rollback Support for Incremental Upgrades"#4604
Future-Outlier wants to merge 2 commits intomasterfrom
revert-4109-rollback-support

Conversation

@Future-Outlier
Copy link
Member

Reverts #4109

see details here
#3209 (comment)

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5eef493d26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Signed-off-by: Future-Outlier <[email protected]>
@Future-Outlier
Copy link
Member Author

cc @ryanaoleary do you mind approve this PR? tks

Copy link
Collaborator

@ryanaoleary ryanaoleary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ryanaoleary
Copy link
Collaborator

ryanaoleary commented Mar 16, 2026

Should note that this isn't a full reversion since we're keeping the change to IsHttpRouteEqual based on the codex comment. We change this function again in #4601 to cover all the fields that aren't set by default by the controller.

Copy link
Contributor

@JiangJiaWei1103 JiangJiaWei1103 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. E2E at #4541 has merged this branch to remove the rollback e2e. I'll merge master again once this PR is merged.

@andrewsykim
Copy link
Member

If we're keeping the feature in alpha status, I suggest keeping new features in the release and then fixing bugs in follow up patches. That's kind of the point of using the alpha feature gate. Unless the bugs are critical and break the entire feature, is that the case?

@Future-Outlier
Copy link
Member Author

Future-Outlier commented Mar 16, 2026

If we're keeping the feature in alpha status, I suggest keeping new features in the release and then fixing bugs in follow up patches. That's kind of the point of using the alpha feature gate. Unless the bugs are critical and break the entire feature, is that the case?

it's ok to keep it, I just think that add the e2e test first, then add back rollback feature might be a cleaner path

@andrewsykim
Copy link
Member

it's ok to keep it, I just think that add the e2e test first, then add back rollback feature might be a cleaner path

Agree, we should have done the e2e tests first, and we definitely should have them before graduating feature to beta in v1.7 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants