-
Notifications
You must be signed in to change notification settings - Fork 11
Use setup bazel gha #166
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
base: main
Are you sure you want to change the base?
Use setup bazel gha #166
Conversation
|
+a:@jwnimmer-tri (trying again after permissions were updated) |
jwnimmer-tri
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.
Fyi In the GHA Actions page, I clicked "re-run all jobs". The re-launched bazel_test job is here: https://github.com/RobotLocomotion/drake-blender/actions/runs/20720901652/job/59585025755. Notice the final output Executed 1 out of 18 tests: 18 tests pass.. That's pretty good (the expensive server tests were cached). It's a bit concerning that there was 1 cache miss (//test:pycodestyle_py_lint_test) but I don't think that's worth investigating.
@jwnimmer-tri reviewed 3 files and all commit messages, and made 3 comments.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri (waiting on @tom-osika).
.github/workflows/main.yml line 48 at r1 (raw file):
- uses: actions/cache/restore@v5 with: path: ~/.cache/pip
In the "install_test" job below, we use setup-python and set "cache: pip" and that gives us pip caching.
It is possible that we could use that same technique here, instead of doing a manual pip cache? That would be a lot simpler.
.github/workflows/main.yml line 74 at r1 (raw file):
# This dumps configuration details to the log. bazelrc: common --announce_rc=yes - name: Report cache sizes
nit Ditto -- we can drop this now.
tom-osika
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 catching that. Its up to you, I can either look into it more before we merge or I can create a ticket for later.
@tom-osika made 2 comments.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri (waiting on @jwnimmer-tri).
.github/workflows/main.yml line 48 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
In the "install_test" job below, we use setup-python and set "cache: pip" and that gives us pip caching.
It is possible that we could use that same technique here, instead of doing a manual pip cache? That would be a lot simpler.
Yes, I think so. According to this, the setup-python action will compute the hash of the dependency file (either pyproject.toml or requirements.txt) and use that as part of the cache key. For pip, the action will cache the global cache directory
Would you like me to switch it?
561ab3d to
0c4d54b
Compare
jwnimmer-tri
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.
@jwnimmer-tri reviewed all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri (waiting on @tom-osika).
.github/workflows/main.yml line 48 at r1 (raw file):
Previously, tom-osika (Tom Osika) wrote…
Yes, I think so. According to this, the
setup-pythonaction will compute the hash of the dependency file (eitherpyproject.tomlorrequirements.txt) and use that as part of the cache key. Forpip, the action will cache the global cache directoryWould you like me to switch it?
Yes, that sounds like a good idea.
f2c1c04 to
6891dad
Compare
e72bb29 to
7bca0a6
Compare
tom-osika
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.
@tom-osika made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri (waiting on @jwnimmer-tri).
.github/workflows/main.yml line 48 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Yes, that sounds like a good idea.
Hmm, for some reason using setup-python caching is causing all of the 18 tests to run instead of the 1/18 we saw before.... Investigating now.
Towards #111
The first run shows the expected cache misses during the
Run bazel-contrib/[email protected]GHA step here. The second run shows cache hits hereJust like RobotLocomotion/models#111, I added a temporary commit modifying the MODULE.bazel to ensure that the repository caching still works even when modifying this file (which, be default, is used as part of the restore key).
This change is