Create a dedicated docker image for ArcticDB development #2086
Conversation
| push: | ||
| paths: | ||
| - 'docker/**' | ||
| - '.github/workflows/dev_docker_image.yml' |
There was a problem hiding this comment.
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 in Docker 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 alternative
There was a problem hiding this comment.
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.
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.
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.
good point , I've added a parameter to override the default tag
| docker push $image_name:$image_ver | ||
|
|
||
| - name: Publish Docker image to GHCR as latest | ||
| if: startsWith(github.ref, 'refs/tags') || github.ref == 'refs/heads/master' |
There was a problem hiding this comment.
Why do we push this for tags?
There was a problem hiding this comment.
copy-paste snafu, I've removed it
|
Please squash the commits when you merge |
Reference Issues/PRs
#2081
What does this implement or fix?
This PR aims to:
Successful manual run of the benchmark flow can be seen here.
It needed to be run manually because the changes to the workflow file are not picked up correctly in the automatic run here.
Any other comments?
Checklist
Checklist for code changes...