-
Notifications
You must be signed in to change notification settings - Fork 67
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
Split build-push workflows #2499
Conversation
push: | ||
paths: | ||
- .github/workflows/build_awshelper.yaml | ||
- Docker/awshelper/** |
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.
sorry, late to the game, but I think only triggers awshelper build on these paths are not enough, since there are many jobs that depends on the awshelper image, and the changes to these job can actually happen in locations other than these two. For example https://github.com/uc-cdis/cloud-automation/blob/master/kube/services/jobs/cedar-ingestion-job.yaml#L57
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 think i understand - if you modify cedar-ingestion-job.yaml
but not the awshelper image, the updated cedar-ingestion-job
can use awshelper:master without a problem, right? You don't need to use awshelper: unless you modified the image?
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.
that job uses a file from here so unless a new awshelper image is being build when changes to that file is merged, then the changes will not always be reflected in the master awshelper image right?
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, i'll send another PR
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.
thank you!!
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.
should we uncheck these 3 actions as required from the branch protection rule?
Split the workflow in 3 so that each image only gets built when changes to the corresponding files are detected.
This way we will not run unnecessary workflows and we will not create unused ECR tags, since there is a limit on how many images we can have in ECR.