Skip to content

Don’t assign the rayv1.Failed to the State field #2258

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

Merged

Conversation

Yicheng-Lu-llll
Copy link
Contributor

@Yicheng-Lu-llll Yicheng-Lu-llll commented Jul 19, 2024

Why are these changes needed?

We should always stop writing rayv1.Failed to the Status.State field. The detailed reason is described here:https://docs.google.com/document/d/1bRL0cZa87eCX6SI7gqthN68CgmHaB6l3-vJuIse-BrY/edit . The new way to indicate failure is described here: https://github.com/ray-project/kuberay/pull/2245/files

Related issue number

Closes #2234

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@Yicheng-Lu-llll Yicheng-Lu-llll force-pushed the no-assign-rayv1-failed-to-state branch from 1b9f64c to e1ab54e Compare July 19, 2024 03:45
@Yicheng-Lu-llll Yicheng-Lu-llll marked this pull request as ready for review July 19, 2024 03:50
@kevin85421 kevin85421 self-assigned this Jul 19, 2024
@Yicheng-Lu-llll Yicheng-Lu-llll force-pushed the no-assign-rayv1-failed-to-state branch from e1ab54e to 8bf92ea Compare July 19, 2024 06:56
Copy link
Member

@kevin85421 kevin85421 left a comment

Choose a reason for hiding this comment

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

  • The feature gate is only used to determine whether to write to the new conditions field. For this PR, we only need to avoid assigning rayv1.Failed to .Status.State.

  • Add a comment to Failed indicating that it is deprecated, but we keep it to avoid compilation errors in projects that import the KubeRay Golang module.

    Failed ClusterState = "failed"

Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>
@Yicheng-Lu-llll Yicheng-Lu-llll force-pushed the no-assign-rayv1-failed-to-state branch from 9c28ce9 to 255c07d Compare July 19, 2024 07:24
Signed-off-by: Yicheng-Lu-llll <luyc58576@gmail.com>
@kevin85421 kevin85421 merged commit ee0a895 into ray-project:master Jul 19, 2024
25 checks passed
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.

[Feature] REP 54: Don’t assign the rayv1.Failed to the State field
2 participants