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.
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
Create a dedicated docker image for ArcticDB development #2086
base: master
Are you sure you want to change the base?
Create a dedicated docker image for ArcticDB development #2086
Changes from 7 commits
36b9ed9
2c3f5fa
3f8740f
03ade9c
79e1407
399b70e
1d5a384
cb303ce
26d4de6
cb43547
1d026dd
4bcb251
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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 don't see the other jobs depend on
Docker Image for Development
job. So changes in the dev docker creation won't be reflected until the image got pushed inDocker Image for Development
job.I feel like the checksum checking method in
cibw_docker_image.yml
for triggering the creation of new docker image a better alternativeThere 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.
We make changes to this docker very infrequently so I prefer to keep the flows a bit simpler and to not add dependencies between them unless it is needed.
Also this image will be used in other repos as well so it has to be tested manually in any case.
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.
Yes I am torn. I understand this docker image won't be changed frequently so it's hard to justify complicating already complicated wheel building process.
At the same time, if any changes made to this docker image, the CI can't tell us whether it works until the change is merged and tag
latest
is updated. Only manual test evidence can be relied on. This kinda defy the purpose of CI.I'll approve it and leave it to @poodlewars for thoughts
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.
It would be nice to be able to run the CI with a candidate new image. How about a run parameter to choose the image tag rather than hardcoding
latest
? Would then be easy to try out a CI run with a candidate image.Or having downstream jobs depend on this one sounds reasonable too. Either way works as long as it's all documented and easy for people to figure out.
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.
good point , I've added a parameter to override the default tag