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

[Serve] Reword no Spot Policy in SkyServe #3684

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Conversation

cblmemo
Copy link
Collaborator

@cblmemo cblmemo commented Jun 24, 2024

Previously, when no spot fallback policy was specified, No spot policy would be shown on the terminal, which is confusing when users are using spot resources. This PR reword it to No spot fallback policy to reduce confusion.

# previously
$ sky serve up -n http examples/serve/http_server/task.yaml --cloud aws --use-spot
Service from YAML spec: examples/serve/http_server/task.yaml
Service Spec:
Readiness probe method:           GET /health
Readiness initial delay seconds:  20
Readiness probe timeout seconds:  15
Replica autoscaling policy:       Fixed 2 replicas
Spot Policy:                      No spot policy
# this PR
$ sky serve up -n http examples/serve/http_server/task.yaml --cloud aws --use-spot
Service from YAML spec: examples/serve/http_server/task.yaml
Service Spec:
Readiness probe method:           GET /health
Readiness initial delay seconds:  20
Readiness probe timeout seconds:  15
Replica autoscaling policy:       Fixed 2 replicas
Spot Policy:                      No spot fallback policy

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • Test in the PR description
  • 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: conda deactivate; bash -i tests/backward_compatibility_tests.sh

@cblmemo cblmemo requested a review from concretevitamin June 24, 2024 03:21
Copy link
Member

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

Seems like we want to rename the LHS column from "Spot Policy:" to "Spot Fallback Policy:" too?

@cblmemo
Copy link
Collaborator Author

cblmemo commented Jun 24, 2024

Seems like we want to rename the LHS column from "Spot Policy:" to "Spot Fallback Policy:" too?

I'm thinking when we later add the spot placer, we might want to reuse this row, which is not "Spot Fallback Policy", so keeping it unchanged now and making the LHS column consistent might reduce confusion. wdyt?

@concretevitamin
Copy link
Member

Sounds good.

@cblmemo cblmemo merged commit ea4506a into master Jun 24, 2024
20 checks passed
@cblmemo cblmemo deleted the reword-serve-spot-policy branch June 24, 2024 14:09
Michaelvll pushed a commit that referenced this pull request Aug 23, 2024
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.

2 participants