-
Notifications
You must be signed in to change notification settings - Fork 539
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
[UX] Add commit hash #2720
[UX] Add commit hash #2720
Conversation
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 adding this! I'm a little confused about the default value {{SKYPILOT_COMMIT_SHA}}
. Where is it replaced by the commit hash? I tried searching in the repository but did not find anything helpful..
__version__ = '1.0.0-dev0' | ||
__root_dir__ = os.path.dirname(os.path.abspath(__file__)) | ||
|
||
# Keep this order to avoid cyclic imports | ||
# pylint: disable=wrong-import-position |
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.
Why add this? Seems like not relevant to this PR..?
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.
Previously, the pylint
did not check this file as the file was not changed for a long time before, and it has wrong-import-position
warning now.
We did not do it in our code, but manually replace it for each release before this PR, and in our CI https://github.com/skypilot-org/skypilot/blob/d38b4b4163e4d5f4ef98895e30b4b5c04fd23dde/.github/workflows/pypi-nightly-build.yml |
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 explanation! LGTM 🫡
We fixed the logic for |
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 fix! Left several comments 😉
Make it possible to get commit hash of the current SkyPilot for both installed from pypi and source code.
Tested (run the relevant ones):
bash format.sh
pip install -e .; cd ~; sky --commit; sky launch -c test --cpus 2+; sky spot launch -n test-aws --cpus 2 --cloud aws
pip install .; cd ~; sky --commit; sky launch -c test --cpus 2+ echo hi; sky down sky-spot-controller-xxx
pip install -i https://test.pypi.org/simple/ skypilot==1.0.0.dev20231020; sky --commit
pip install -e .; sky launch -c test-commit --cpus 2 echo hi; ssh test-commit; sky -c
pytest tests/test_smoke.py
pytest tests/test_smoke.py::test_fill_in_the_name
bash tests/backward_comaptibility_tests.sh