-
-
Notifications
You must be signed in to change notification settings - Fork 553
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
Multi-architecture Docker images and parallel builds #3430
Conversation
pybamm-team#3316 fix discrepancy with tagging the images, add a step to append tags to GitHub output, add support for multi-architecture images by setting up QEMU.
I opened this PR from @arjxn-py's fork because that repository has credentials for Docker Hub. Currently, the ALL and the JAX images are going to fail (logs) because conda fails to find a suitable version of |
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3430 +/- ##
========================================
Coverage 99.58% 99.58%
========================================
Files 256 256
Lines 19998 20003 +5
========================================
+ Hits 19915 19920 +5
Misses 83 83 ☔ View full report in Codecov by Sentry. |
So I did some digging and found this: jax-ml/jax#13608 (comment) which sheds some info. One needs to build |
e275775
to
34b7288
Compare
I realised that specifying the platforms in a matrix might overwrite images depending on which image and platform build gets completed (and thereby pushed) first. For now, adding conditional steps looks like an apt solution to control what images are built and pushed. |
Are these images compatible with other platforms as well? |
Yes, pulling the image makes things work natively since it auto-detects my platform (so I don't have to specify P.S. I actually forgot to un-set |
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.
Looks amazing, thanks, @agriyakhetarpal! I'll request @arjxn-py's review before merging this.
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 @agriyakhetarpal, looks awesome.
But when i see the images at docker hub, it looks like that the latest
image is getting updated instead of all
image.
Yes, this workflow when merged will update the images and fix that issue (already identified in #3316 (comment)) |
Best then, happy to go forward with this 🚀 |
Thanks for the review @arjxn-py; the workflow won't run in forks now, but could you un-set the secrets in your repository settings just in case? It would be good for security reasons, say, lest a password breach occurs. No repository outside this one should have them to minimise chances of unauthorised access. |
Sure I'll do that. |
Description
This PR uses QEMU to build images for aarch64/arm64 platforms besides amd64, and fixes the error in #3316 (comment) about an improper tag for one of the images by using a build-matrix setup to create images. Related to #3312.
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all
(or$ nox -s tests
)$ python run-tests.py --doctest
(or$ nox -s doctests
)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick
(or$ nox -s quick
).Further checks: