Skip to content
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

Make sky spot launch default -r/--retry-until-up to True. #1781

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

concretevitamin
Copy link
Collaborator

From user feedback, it's unintuitive that a managed spot job by default don't have this flag to True. Forgetting to set this flag made the job fail with FAILED_NO_RESOURCES.

Tested (run the relevant ones):

  • Any manual or new tests for this PR (please specify below)
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Thanks for the change @concretevitamin! Since we have made sure pre-check and user setup failure will fail early in our recent PRs, I think it should be fine to make this retry-until-up default now.

sky/cli.py Show resolved Hide resolved
@concretevitamin concretevitamin merged commit 3a49bc8 into master Apr 13, 2023
@concretevitamin concretevitamin deleted the retry branch April 13, 2023 05:51
@Michaelvll Michaelvll mentioned this pull request May 31, 2023
9 tasks
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.

None yet

2 participants