-
Notifications
You must be signed in to change notification settings - Fork 0
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
Improved GHA CI build and local dev DX #32
Conversation
victorskl
commented
Jun 21, 2023
- Bumped localstack v2
- Added reusable GHA container image caches.yml workflow
- Added mocking section in README
.github/workflows/caches.yml
Outdated
uses: actions/cache@v3 | ||
with: | ||
path: ci/cache/docker/localstack | ||
key: cache-docker-localstack-2.1 |
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.
can you drag this "2.1" somehow up into a const or env variable.. its just that it then is used in about 10 different spots in the next 10 lines - easy for someone to miss changing it when bumping the version..
.github/workflows/caches.yml
Outdated
uses: actions/cache@v3 | ||
with: | ||
path: ci/cache/docker/mysql | ||
key: cache-docker-mysql-8 |
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.
same for the myself 'version'
.github/workflows/caches.yml
Outdated
# HAProxy | ||
- name: Restore HAProxy Image Cache if it exists | ||
id: cache-docker-haproxy | ||
uses: actions/cache@v3 | ||
with: | ||
path: ci/cache/docker/haproxy | ||
key: cache-docker-haproxy-2.4 | ||
|
||
- name: Update HAProxy Image Cache if cache miss | ||
if: steps.cache-docker-haproxy.outputs.cache-hit != 'true' | ||
run: docker pull public.ecr.aws/docker/library/haproxy:2.4 && mkdir -p ci/cache/docker/haproxy && docker image save public.ecr.aws/docker/library/haproxy:2.4 --output ./ci/cache/docker/haproxy/haproxy-2.4.tar | ||
|
||
- name: Use HAProxy Image Cache if cache hit | ||
if: steps.cache-docker-haproxy.outputs.cache-hit == 'true' | ||
run: docker image load --input ./ci/cache/docker/haproxy/haproxy-2.4.tar |
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.
in fact - is it possible to refactor this into a repeatably useable snippet (I believe github actions allows 'functions' of some sort)
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.
oh - I see the whole thing is already a re-usable script.. all good
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.
Ah yes. But you are also right. Let me experiment with composite action in another feat branch...
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.
Yup. It works better with refactoring into reusable composite action instead. Could you check latest again when you are able, pls. I reckon, it looks better now.
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.
FYI. Corresponding build is here and pass.
https://github.com/umccr/orcabus/actions/runs/5333413263/jobs/9663842683?pr=32
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.
Gentle ping @andrewpatto
* Bumped localstack v2 * Added reusable GHA container image caches.yml workflow * Added mocking section in README
* It seems like image caches are not carried across different job process. Therefore, simplifying it into single workflow where this should work better.
fe49030
to
33426c9
Compare