Improve detection of when breeze CI image needs rebuilding #33603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously we have been using provider.yaml file modification as a sign that the docker image needs rebuilding when starting image. However just modification of provider.yaml file is not a sign that the image needs rebuilding. The image needs rebuilding when provider dependencies changed, but there are many more reasons why provider.yaml file changed - especially recently provider.yaml file contains much more information and dependencies are only part of it. Provider.yaml files can also be modified by release manager wnen documentation is prepared, but none of the documentation change is a reason for rebuilding the image. This PR optimize the check for image building introducing two step process: * first we check if provider.yaml files changed * if they did, we regenerate provider dependencies by manully running the pre-commit script * then provider_dependencies.json is used instead of all providers to determine if the image needs rebuilding This has several nice side effects: * the list of files that have been modified displayed to the user is potentially much smaller (no provider.yaml files) * provider_dependencies.json is regenereated automatically when you run any breeze command, which means that you do not have to have pre-commit installed to regenerate it * the notification "image needs rebuilding" will be printed less frequently to the user - only when it is really needed * preparing provider documentation in CI will not trigger image rebuilding (which might occasionally fail in such case especially when we bring back a provider from long suspension like it happened in apache#33574
potiuk
requested review from
Taragolis,
pankajkoti,
hussein-awala,
amoghrajesh and
pankajastro
amoghrajesh
approved these changes
Aug 22, 2023
pankajkoti
approved these changes
Aug 22, 2023
potiuk
commented
Aug 22, 2023
Taragolis
approved these changes
Aug 22, 2023
potiuk
added a commit
that referenced
this pull request
Aug 25, 2023
* Improve detection of when breeze CI image needs rebuilding Previously we have been using provider.yaml file modification as a sign that the docker image needs rebuilding when starting image. However just modification of provider.yaml file is not a sign that the image needs rebuilding. The image needs rebuilding when provider dependencies changed, but there are many more reasons why provider.yaml file changed - especially recently provider.yaml file contains much more information and dependencies are only part of it. Provider.yaml files can also be modified by release manager wnen documentation is prepared, but none of the documentation change is a reason for rebuilding the image. This PR optimize the check for image building introducing two step process: * first we check if provider.yaml files changed * if they did, we regenerate provider dependencies by manully running the pre-commit script * then provider_dependencies.json is used instead of all providers to determine if the image needs rebuilding This has several nice side effects: * the list of files that have been modified displayed to the user is potentially much smaller (no provider.yaml files) * provider_dependencies.json is regenereated automatically when you run any breeze command, which means that you do not have to have pre-commit installed to regenerate it * the notification "image needs rebuilding" will be printed less frequently to the user - only when it is really needed * preparing provider documentation in CI will not trigger image rebuilding (which might occasionally fail in such case especially when we bring back a provider from long suspension like it happened in #33574 * Update dev/breeze/src/airflow_breeze/commands/developer_commands.py (cherry picked from commit ac0d5b3)
97 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously we have been using provider.yaml file modification as a sign that the docker image needs rebuilding when starting image. However just modification of provider.yaml file is not a sign that the image needs rebuilding. The image needs rebuilding when provider dependencies changed, but there are many more reasons why provider.yaml file changed - especially recently provider.yaml file contains much more information and dependencies are only part of it. Provider.yaml files can also be modified by release manager wnen documentation is prepared, but none of the documentation change is a reason for rebuilding the image.
This PR optimize the check for image building introducing two step process:
This has several nice side effects:
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.