[Core] Add sky jobs logs --name and fix sky job logs spinner
#3538
Conversation
concretevitamin
requested review from
romilbhardwaj
and removed request for
concretevitamin
sky/cli.py
Outdated
| if name is not None: | ||
| job_queue = managed_jobs.queue(refresh=False) | ||
| job_id = None | ||
| for job in job_queue: | ||
| if job['job_name'] == name: | ||
| job_id = job['job_id'] | ||
| break | ||
| if job_id is None: | ||
| click.echo(f'Managed job with name {name!r} not found.') | ||
| sys.exit(1) |
There was a problem hiding this comment.
To discuss: will this add too much overhead to the sky jobs logs? Now it requires two SSH to the controller IIUC. Also, this seems not used in the smoke test?
There was a problem hiding this comment.
This might be fine as the code path will only be entered when name is specified which is currently only used in smoke test?
There was a problem hiding this comment.
If a user does sky jobs logs --controller -n train-job, then it will also hit this code path?
Michaelvll
changed the title
sky jobs logs --name and fix sky job logs spinner
There was a problem hiding this comment.
Thanks for adding this @Michaelvll ! Left some discussions :))
sky/jobs/utils.py
Outdated
| from sky.jobs import utils | ||
| from sky.jobs import constants as managed_job_constants | ||
| from sky.jobs import state as managed_job_state | ||
| from typing import Optional |
There was a problem hiding this comment.
This is needed as we inspect.getsource the function stream_logs, which uses Optional in its signature. Moved this import to the later place.
This adds support for
sky jobs logs --name, fixes #3599, and add test forsky job logsThis is to prevent issue like #3531 with the smoke test.
Tested (run the relevant ones):
bash format.shsky jobs launch -n test-log --cloud aws --gpus H100:8 echo hiand check the output of the spinnersky launch -c test-output --cloud aws task.yamlsky launch -c test-output --cloud gcp task.yamlpytest tests/test_smoke.pypytest tests/test_smoke.py::test_fill_in_the_namepytest tests/test_smoke.py::test_managed_jobspytest tests/test_smoke.py::test_managed_jobs --kubernetespytest tests/test_smoke.py::test_minimalconda deactivate; bash -i tests/backward_compatibility_tests.sh