-
Notifications
You must be signed in to change notification settings - Fork 25
ci: docker push (-> ghcr) #314
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?
Conversation
|
I've decided to allow images to specify architectures, and make some architecture support a burden of the algorithm Dockerfile wrapper instead. |
Documentation build overview
Show files changed (2 files in total): 📝 2 modified | ➕ 0 added | ➖ 0 deleted
|
agitter
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.
I took an initial pass and like @jhiemstrawisc's opinions as well.
This change would mean we are no longer pushing new images to our DockerHub organization. Are there consequences to that? SPRAS doesn't much of an external userbase, so perhaps not yet. If it did, we may be abandoning users who couldn't get new images that newer versions of the SPRAS package required at some point in the future.
In terms of versioning, Justin and I originally discussed how the Docker images aren't tied to a specific SPRAS release. That's why we started an integer versioning convention. There is some implicit relationship where pathway reconstruction algorithm vX is only compatible with SPRAS >= vY. Ideally that would be explicit.
That versioning still makes sense to me because users need to know what version of a pathway algorithm image they are using and may need to change that version independently of the SPRAS package version. Even if we switch registries, I don't think we should reset versions at v1.
I would also like some way to document release notes for each Docker image as we iterate on them. That is currently done in the readmes. Should we keep it there?
Making this change would add another step to the contributing guide.
As you've mentioned, this would only affect current users that have not changed their configured container registry to be
I don't believe this case would happen, but I do see the rest of the motivation for decoupling the docker image versions from the SPRAS versions - also possibly because seeing a different SPRAS version appear in errors for docker containers can be misleading.
This is fine, we just need to be consistent about it. It might be a good idea to begin making all of the container READMEs consistent in the first place. |
|
I left this PR decaying - @jhiemstrawisc I changed the versions, with the exception of AllPairs which had an undocumented I've also updated |
This PR makes it easy to push docker images. This also gives us a static list of what architectures we support, closing #272.
Since this is a little more inconvenient to do with the Docker Registry, we continue with the GHCR (closes #77), and create a new workflow that allows maintainers to deploy containers on certain versions.
To make this convenient, a
metadata.jsonfile is added to every singledocker-wrapper(should we lint this?) which specifies what the actual desired docker name is.The versioning comments have been addressed. Original comment: