-
Notifications
You must be signed in to change notification settings - Fork 854
[Jobs] Add huggingface-cli jobs commands
#3211
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
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
This is ready for review for the launch in the coming days ! Would be cool to do a release right after we merge Btw I integrated your addition @davanstrien from lhoestq/hfjobs#8 and added some useful uv options: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine as an experiment, but not a huge fan of the local file uploading to a remote repo..
Is there any way to either:
- pass the file content as an argument (string) to
uv(and thus to the Jobs creation API) - ask the infra team to add a new feature to the Jobs creation API where you can a dict of file name to file contents and they are exposed to the docker command? (not sure if it's feasible @christophe-rannou)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pass the file content as an argument (string) to uv (and thus to the Jobs creation API)
I don't think this is directly possible in UV at the moment.
ask the infra team to add a new feature to the Jobs creation API where you can a dict of file name to file contents and they are exposed to the docker command? (not sure if it's feasible @christophe-rannou)
Think this would be nice if it was possible. @christophe-rannou, would this be difficult to implement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of the logic of doing using a repo as a backend was to open up options to explore approaches where you could do something like
huggingface-cli jobs uv run --from-repo davanstrien/nice-data-generation-pipeline
I think for that to fully make sense, it would probably also be better to have a "generic" or "code" repo type rather than using a dataset as the storage repo.
Co-authored-by: Julien Chaumond <[email protected]>
|
As I high-level comment, it'd be good to have all the API logic added to |
I can take care of this for tomorrow |
hanouticelina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice UX, I like it! 🔥 i left some initial comments.
As mentioned by @Wauplin, let's centralize the logic into HfApi:
HfApi.run_job(...)
HfApi.list_jobs(...)
HfApi.inspect_job(...)
HfApi.cancel_job(...)
HfApi.fetch_job_logs(...)that way, the CLI subcommands become lightweight wrappers and maybe we can put all the sub parsers (run, ps, inspect, logs, cancel, uv) inside one src/huggingface_hub/commands/jobs.py file.
|
I took your comments into account and added I also added the documentation page |
|
Imo it would be good to still have support for passing an image when using uv run. Many ai focused images have uv included now and it can make the setup much more reliable i.e. for vllm it works out of the box using their default image. If we pass the astral image there is still a lot more steps to get things working smoothly |
|
I added the |
|
I also just pushed a commit to use |
Wauplin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally approved ✔️ 😄
Thanks a lot @lhoestq @davanstrien for the work on this tool! 🔥
|
exclude |
Good call, removed in e6043ae |
Wauplin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
thanks for the reviews :) merging once the CI is green (edit: and also after a final commit of a missing docstring) |
| Usage: | ||
| # run a job | ||
| huggingface-cli jobs run image command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) expanding the top level docstring to list the other jobs sub commands
(cc @Wauplin, since the PR has been merged, we can do that in the follow-up PR that will switch huggingface-cli jobs -> hf jobs)
| Usage: | |
| # run a job | |
| huggingface-cli jobs run image command | |
| Usage: | |
| # run a job | |
| huggingface-cli jobs run <image> <command> | |
| # List running or completed jobs | |
| huggingface-cli jobs ps [-a] [-f key=value] [--format TEMPLATE] | |
| # Stream logs from a job | |
| huggingface-cli jobs logs <job-id> | |
| # Inspect detailed information about a job | |
| huggingface-cli jobs inspect <job-id> | |
| # Cancel a running job | |
| huggingface-cli jobs cancel <job-id> | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in #3250
from https://github.com/lhoestq/hfjobs