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

CLI: Deprecate cpunode/gpunode/tpunode, hide admin #2800

Merged
merged 5 commits into from
Nov 18, 2023

Conversation

concretevitamin
Copy link
Collaborator

Changes

  • Deprecate + hide cpunode/gpunode/tpunode
    • Users/we have found them confusing, since sky launch essentially does the same thing without the final ssh
  • Purge mentions of the above in docs
  • Hide admin
  • Minor rewording of docstrs

Now

» sky gpunode                 
WARNING: `sky gpunode` is deprecated and will be removed in a future release. Please use `sky launch --gpus <gpus>` instead.

I 11-17 09:23:31 optimizer.py:694] == Optimizer ==
I 11-17 09:23:31 optimizer.py:706] Target: minimizing cost
I 11-17 09:23:31 optimizer.py:717] Estimated cost: $0.2 / hour
...

» sky                           
...
Commands:
  launch       Launch a cluster or task.
  exec         Execute a task or command on an existing cluster.
  status       Show clusters.
  cost-report  Show estimated costs for launched clusters.
  queue        Show the job queue for cluster(s).
  logs         Tail the log of a job.
  cancel       Cancel job(s).
  stop         Stop cluster(s).
  autostop     Schedule an autostop or autodown for cluster(s).
  start        Restart cluster(s).
  down         Tear down cluster(s).
  check        Check which clouds are available to use.
  show-gpus    Show supported GPU/TPU/accelerators and their prices.
  storage      SkyPilot Storage CLI.
  bench        SkyPilot Benchmark CLI.
  spot         Managed Spot CLI (spot instances with auto-recovery).
  serve        SkyServe CLI (multi-region, multi-cloud serving).

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below): above; rendered docs
  • 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 deprecating the commands @concretevitamin! LGTM.

docs/source/examples/gpu-jupyter.rst Show resolved Hide resolved
@@ -48,7 +50,7 @@ More details can be found on GCP `documentation <https://cloud.google.com/tpu/do
TPU VMs
-------

To use TPU VMs, set the following in a task YAML's ``resources`` field:
To use TPU VMs, set the following in a task YAML's ``resources`` field:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: in the future, we probably need to add the --tpu-vm for sky launch as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or we can switch the default from tpu nodes to tpu vms @infwinston

concretevitamin and others added 3 commits November 17, 2023 11:38
Co-authored-by: Zhanghao Wu <zhanghao.wu@outlook.com>
Copy link
Collaborator

@romilbhardwaj romilbhardwaj left a comment

Choose a reason for hiding this comment

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

Comment on lines +1208 to +1217
def _deprecate_and_hide_command(group, command_to_deprecate,
alternative_command):
"""Hide a command and show a deprecation note, hinting the alternative."""
command_to_deprecate.hidden = True
if group is not None:
orig = f'sky {group.name} {command_to_deprecate.name}'
else:
orig = f'sky {command_to_deprecate.name}'
command_to_deprecate.invoke = _with_deprecation_warning(
command_to_deprecate.invoke, alternative_command, orig)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we consider implementing this as a method decorator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Either way seems fine to me. This is following L1195 and https://google.github.io/styleguide/pyguide.html#217-function-and-method-decorators :)

@concretevitamin concretevitamin merged commit 3a7c858 into master Nov 18, 2023
19 checks passed
@concretevitamin concretevitamin deleted the cli-docs-hide branch November 18, 2023 04:37
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

3 participants