refactor: unify container building/testing - #1128
Open
paperbenni wants to merge 11 commits into
Open
Conversation
Contributor
Author
|
new ruff is way more strict, this has been fixed in #1115 |
bring this more in line with other images, dont push to temporary repos which are private
also fixes an issue where the local instead of pushed image was tested
remove unneeded param
paperbenni
changed the title
Member
|
This looks mostly good to me, ruff is unhappy, can you make it happy. |
Member
|
@aliciaaevans do you want to have a look, or merge after fixing ruff? |
Contributor
Author
I can, but it would basically be cherry picking thousands of lines from #1115 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
All container build workflows were nearly identical and included some drift.
Some of the tests also accidentally tested a local temporary image instead of the claimed remote image.
The cos7 image being inconsistent with the rest of the building workflows also caused it to attempt to push to a private repo.
Also only one of the Dockerfile.test files was actually repeatedly tested, with the other ones testing the incorrect image.
It was possible a bug would pass PR tests but fail release CI (or get released)
What
Extract duplicated build logic into a shared action and a shared test script.
This PR also removes the superseded responder image, (bot image is the one used nowadays)
All images now use multi-arch manifests instead of architecture specific suffixes.
This means imagename-aarch64 is no longer valid, and instead all architectures simply use imagename
This will enable (and require) simplifying the bioconda-recipes workflows which use conditional image names
Fixes the bug testing the wrong image so that now all Dockerfile.test file are actually used with their correct base images