Skip to content
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

Simplifies check whether the CI image should be rebuilt #12181

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Nov 8, 2020

Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@potiuk
Copy link
Member Author

potiuk commented Nov 8, 2020

Together with just merged #12177 we should finally handle all the cases where Breeze correctly detects when it is faster to build image locally or whether to pull it first.

@potiuk potiuk force-pushed the simplify-manifest-check-for-image-rebuild branch from cbdcc80 to e67bdc9 Compare November 9, 2020 14:41
@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the simplify-manifest-check-for-image-rebuild branch 2 times, most recently from ab94da1 to 1971f27 Compare November 9, 2020 20:25
@potiuk
Copy link
Member Author

potiuk commented Nov 11, 2020

I would love to get that in and work today on rock-solid image pulling. This is the last part of what I wanted to do with simplifying the detection of "pull vs. build" and with this in place I think I will finally be able to solve some of the remaining unnecessary rebuilts of Breeze. Looking forward to some reviews :)

@potiuk potiuk force-pushed the simplify-manifest-check-for-image-rebuild branch from 1971f27 to 359fc6c Compare November 12, 2020 11:32
Dockerfile.ci Show resolved Hide resolved
scripts/ci/libraries/_build_images.sh Outdated Show resolved Hide resolved
@potiuk
Copy link
Member Author

potiuk commented Nov 12, 2020

Thanks @feluelle ! Seems I am getting worse with the typos :D

@potiuk
Copy link
Member Author

potiuk commented Nov 13, 2020

Hey @feluelle. All Good it seems!

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 13, 2020
@github-actions
Copy link

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

@potiuk potiuk force-pushed the simplify-manifest-check-for-image-rebuild branch 2 times, most recently from da4df98 to 490c469 Compare November 13, 2020 14:22
@potiuk
Copy link
Member Author

potiuk commented Nov 13, 2020

Hey @feluelle -> i did a few more modifications to make it even clearer to know what's going on and double-checked what happens for migration for those who have the current image/manifest. It should be rather smooth migration now :).

@potiuk potiuk requested a review from feluelle November 13, 2020 14:24
@potiuk potiuk force-pushed the simplify-manifest-check-for-image-rebuild branch from 490c469 to 41e5860 Compare November 13, 2020 14:26
Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.
@potiuk potiuk force-pushed the simplify-manifest-check-for-image-rebuild branch from 41e5860 to 2d639d9 Compare November 13, 2020 18:58
@potiuk potiuk merged commit 167b9b9 into apache:master Nov 13, 2020
@potiuk potiuk deleted the simplify-manifest-check-for-image-rebuild branch November 13, 2020 21:21
potiuk added a commit that referenced this pull request Nov 14, 2020
Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.

(cherry picked from commit 167b9b9)
@potiuk potiuk added this to the Airflow 1.10.13 milestone Nov 14, 2020
@potiuk potiuk added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 14, 2020
potiuk added a commit that referenced this pull request Nov 16, 2020
Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.

(cherry picked from commit 167b9b9)
potiuk added a commit that referenced this pull request Nov 16, 2020
Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.

(cherry picked from commit 167b9b9)
kaxil pushed a commit that referenced this pull request Nov 18, 2020
Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.

(cherry picked from commit 167b9b9)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
Rather than counting changed layers in the image (which was
enigmatic, difficult and prone to some magic number) we rely now
on random file generated while building the image.

We are using the docker image caching mechanism here. The random
file will be regenerated only when the previous layer (which is
about installling Airflow dependencies for the first time) gets
rebuild. And for us this is the indication, that the building
the image will take quite some time. This layer should be
relatively static - even if setup.py changes the CI image is
designed in the way that the first time installation of Airflow
dependencies is not invalidated.

This should lead to faster and less frequent rebuild for people
using Breeze and static checks.

(cherry picked from commit 167b9b9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants