Skip to content
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

Separate base image build #105

Merged
merged 83 commits into from
Sep 22, 2023
Merged

Separate base image build #105

merged 83 commits into from
Sep 22, 2023

Conversation

anayden
Copy link
Collaborator

@anayden anayden commented Aug 30, 2023

No description provided.

@anayden anayden changed the base branch from main to develop August 30, 2023 17:48
@github-actions
Copy link

github-actions bot commented Aug 30, 2023

Docker image tag(s) pushed:

rnacentral/r2dt:pr-105

Labels added to images:

org.opencontainers.image.created=2023-09-22T08:30:43.043Z
org.opencontainers.image.description=Visualise RNA secondary structure in consistent, reproducible and recognisable layouts
org.opencontainers.image.licenses=Apache-2.0
org.opencontainers.image.revision=90e9fc1fa45d68c5e2de3bef34ad0511b2db7b34
org.opencontainers.image.source=https://github.com/RNAcentral/R2DT
org.opencontainers.image.title=R2DT
org.opencontainers.image.url=https://github.com/RNAcentral/R2DT
org.opencontainers.image.version=pr-105

@anayden
Copy link
Collaborator Author

anayden commented Aug 30, 2023

@AntonPetrov addressing your comment from the original PR:

  1. The build issues have been resolved
  2. Overall, I think it would be a bad practice to have changes to both base and main image in the same PR. Also, rebuilding the base image every time, even if it's not changed, seems redundant. I think the approach should be: set a specific (not latest) base image version in the Dockerfile of the primary image; when when changes to the base image needed, release a new version in an isolated PR; then migrate Dockerfile to the new version of the base image and create a PR for the primary image only. In this scenario, there should be no dependencies between the base and primary images. In fact, this PR should ideally be the only one when both builds are triggered.

Hope this helps.

@AntonPetrov
Copy link
Member

@anayden Awesome, thanks! I will test out the workflow tomorrow - I will push a small change in the base image (that will make it ~1Gb smaller 🎉) and will see if the image builds automatically etc.

I like the idea of using a specific version of base image and not the latest tag, and I do agree that having 2 images is the way to go. However, I am trying to think through some development scenarios. For example, let's say that a new version of the Traveler software (which is part of the base image) implements a new command line option that needs to be added to the python code. What would be the correct procedure for creating an atomic PR using the multi-stage build approach? 🤔

The code makes sense and we are almost there - I am just trying to wrap my head around the new way of doing things. Thanks again for your excellent contributions - I am sure it turned out to be more work than originally thought! 🐰🕳️

@AntonPetrov AntonPetrov self-assigned this Aug 30, 2023
@anayden
Copy link
Collaborator Author

anayden commented Aug 31, 2023

@AntonPetrov thank you for your kind words!

For example, let's say that a new version of the Traveler software (which is part of the base image) implements a new command line option that needs to be added to the python code. What would be the correct procedure for creating an atomic PR using the multi-stage build approach? 🤔

Basically, we have 2 atomic changes here:

  1. Create a PR with a new version of the Traveler software and tag it as r2dt-base:v1.4.1.
  2. The new image is built and pushed. Now you would have r2dt-base:v1.4 and r2dt-base:v1.4.1
  3. In a new branch, update Dockerfile of the main image to source r2dt-base:v1.4.1 and use the new command line option in the code. Create a new PR, that would build a new version of r2dt (not necessarily a release, this can be an intermediate "dev" version as well).

So basically updating the base image and update the client of the base image (the app itself) would be 2 separate atomic operations in separate branches with separate PRs, one made after another is approved and merged.

I can also make both workflows part of a single one, so that you would always build base image + app image, but I think that would make builds much slower as base image rebuilds are supposed to be rare (padme-meme.jpg). But I'm fine either way, it's your call for sure :)

@AntonPetrov AntonPetrov merged commit d122951 into develop Sep 22, 2023
3 checks passed
@AntonPetrov AntonPetrov deleted the separate-base-image branch September 22, 2023 08:38
@AntonPetrov
Copy link
Member

Merged now, thank you so much @anayden! 🎈🎉🚀

@AntonPetrov AntonPetrov mentioned this pull request Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants