Add core programmatic API align with CLI

[Michaelvll]

Closes #871 and #980. Also, makes #972 easier.

Our current implementation has many functionalities in the cli.py, which makes python API users have to use subprocess to do the cluster, jobs, and storage management, and cannot handle easily handle the return values (needs to be done with string process).

This PR currently just has the skeleton of SDK and APIs. The implementation will be filled later. Please take a look at the API hierarchy and let me know if you find it not appropriate. : )

I am not sure whether the stop, start, down, autostop should be placed under sky or sky.sdk. If placed under sky.sdk, it is a bit weird to only have launch and exec under sky.

I updated the hierarchy of the APIs based on the suggestion of @romilbhardwaj.

Other changes

1. Added a smoke test for the sky logs.

Hierarchy of the APIs:

sky

• sky.launch
• sky.exec
• sky.stop
• sky.start
• sky.down
• sky.autostop
• sky.status
• sky.queue
• sky.cancel
• sky.spot_status
• sky.spot_cancel
• sky.get_storage
• sky.delete_storage

TODOs

• Add docs for python API (Add document for the API #1046 )

Tested:

CLIs

• sky launch -c min ./example/minimal.yaml
• sky stop min
• sky start min
• sky autostop min
• sky autostop --cancel min
• sky queue
• sky spot launch -n test-queue 'sleep 10000; echo hi'
• sky spot status
• sky spot cancel -n test-queue
• sky logs test-multi-echo-zhwu-164b 1 2 3 4 -s
• sky logs test-multi-echo-zhwu-164b 3 -s
• sky logs test-multi-echo-zhwu-164b 3
• sky logs test-multi-echo-zhwu-164b 1 2 3 4 fails
• tests/run_smoke_test.sh (Failed TPU VM as TPU VM: guard against spurious and transient failures #962)

[romilbhardwaj]

Thanks for sharing the draft of the PR for review!

I am not sure whether the stop, start, down, autostop should be placed under sky or sky.sdk. If placed under sky.sdk, it is a bit weird to only have launch and exec under sky.

Another way to achieve our goal is to have each functionality implemented in its module instead of sdk.py (e.g., storage functions go in sky.storage, spot functions go in sky.spot etc.). Functions which cannot be put in a specific module go in sdk.py. All methods get imported in sky's __init__.py, so they are directly accessible with sky.xyz.

So our __init__.py would look like:

from sky.execution import launch
from sky.execution import exec
from sky.execution import stop
from sky.execution import start
from sky.execution import down
from sky.sdk import autostop # Should it be in sky.execution? idk
from sky.sdk import status
from sky.sdk import queue
from sky.sdk import cancel
from sky.spot import spot_status
from sky.spot import spot_cancel
from sky.storage import get_storage
from sky.storage import delete_storage

wdyt?

[concretevitamin]

(have not read the PR; just a note)

This is timely! @pounde reported that there's no simple API to call for the effects of sky down. The current alternative is, I think, too verbose:

from sky import backends
from sky import global_user_state

handle = global_user_state.get_handle_from_cluster_name(CLUSTER)
backends.CloudVmRayBackend().teardown(handle, terminate=True)

[Michaelvll]

Thanks for sharing the draft of the PR for review!

I am not sure whether the stop, start, down, autostop should be placed under sky or sky.sdk. If placed under sky.sdk, it is a bit weird to only have launch and exec under sky.

Another way to achieve our goal is to have each functionality implemented in its module instead of sdk.py (e.g., storage functions go in sky.storage, spot functions go in sky.spot etc.). Functions which cannot be put in a specific module go in sdk.py. All methods get imported in sky's __init__.py, so they are directly accessible with sky.xyz.

So our __init__.py would look like:

from sky.execution import launch
from sky.execution import exec
from sky.execution import stop
from sky.execution import start
from sky.execution import down
from sky.sdk import autostop # Should it be in sky.execution? idk
from sky.sdk import status
from sky.sdk import queue
from sky.sdk import cancel
from sky.spot import spot_status
from sky.spot import spot_cancel
from sky.storage import get_storage
from sky.storage import delete_storage

wdyt?

Good idea! The only drawback of this would be there might be too many functions under sky, not sure if that will cause any confusion (or a bit overwhelming for the users to look at the APIs, please feel free to push back). Do you think it would be better to make spot_launch as sky.spot.launch and delete_storage as sky.storage.delete?
After thinking more about that, I feel like my proposal is a bad idea, as that will cause many unnecessary API exposures from sky.spot and sky.storage.

[Michaelvll]

Merged the master branch.
Tested:

• ./tests/run_smoke_tests.sh
• pytest ./tests/test_onprem.py

For the onprem and benchmark, I have not moved them into the core as they may not be immediately needed yet, and may experience some API change in the short future. We can move them in a future PR. Wdyt @romilbhardwaj @concretevitamin ?

Future TODO:

• Move the Onprem and Benchmark CLI into sdk.

[concretevitamin]

For the onprem and benchmark, I have not moved them into the core as they may not be immediately needed yet, and may experience some API change in the short future. We can move them in a future PR. Wdyt @romilbhardwaj @concretevitamin ?

This sounds good to me.

[romilbhardwaj]

This is great! We will have a much cleaner python API after this PR. Left some comments

[romilbhardwaj]

comment: It would be nice to have a flag to specify the log download destination. They get put into ~/sky_logs/, which is okay for now, but something we should keep an eye out for (or mark as a good first issue ;) )

[Michaelvll]

That is a great point! I just added the option for the core.download_logs, so that the user can specify the local_dir. For the CLI, it may be better to keep it simpler without the option?

[romilbhardwaj]

lgtm, thanks @Michaelvll! Should be good to merge after smoke tests. We should also add examples for the new APIs as a new PR

[Michaelvll]

Thank you for the review @romilbhardwaj! Just passed the smoke tests. Merging.