Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jul 12, 2025

The #53212 changed the quick-image-build check to only run on canary build, but this was not the intention - and the image started to fail because of timeout minutes were too short after we added python building from sources.

This PR fixes it "properly" - changes timeout minutes to be slightly longer than the timeout (900 seconds) we specify in build command and brings back building the image on regular PRs.


^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch labels Jul 12, 2025
@potiuk potiuk requested review from aritra24 and kaxil July 12, 2025 08:19
@gopidesupavan
Copy link
Member

@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2025

https://github.com/apache/airflow/pull/53227/files#diff-321772ef8ca2bdcf13445925a5fadc9f7879a64fc2fe3443b45bbd03a2ba2185R151 is docker login works on fork?

Good point - I think it does somehow (it did work so far). But we can remove the whole step here - it's not needed at all

The apache#53212 changed the quick-image-build check to only run on
canary build, but this was not the intention - and the image started
to fail because of timeout minutes were too short after we added
python building from sources.

This PR fixes it "properly" - changes timeout minutes to be slightly
longer than the timeout (900 seconds) we specify in build command
and brings back building the image on regular PRs.
@potiuk potiuk force-pushed the fix-timeout-minutes-on-quick-image-build branch from 94aed4a to de4852d Compare July 12, 2025 08:26
@gopidesupavan
Copy link
Member

https://github.com/apache/airflow/pull/53227/files#diff-321772ef8ca2bdcf13445925a5fadc9f7879a64fc2fe3443b45bbd03a2ba2185R151 is docker login works on fork?

Good point - I think it does somehow (it did work so far). But we can remove the whole step here - it's not needed at all

cool was chnaged it because thought it needs docker login :)

@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2025

Yep.. It somehow worked in the PR that failed and triggered #53212

Screenshot 2025-07-12 at 10 26 51

@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2025

But I removed it now.. The "build" only builds local image it does not try to push it, it only makes sure it takes less than <900 seconds - so we don't even need to log-in to docker

@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2025

But ... it might also hit rate limits. Maybe we will have to bring it back - let's see. I believe anything coming from GitHub CI to docker has higher rate limits by default as far as I remember.

@aritra24
Copy link
Collaborator

Thanks for the change!

@potiuk potiuk merged commit 5579edd into apache:main Jul 12, 2025
102 checks passed
@potiuk potiuk deleted the fix-timeout-minutes-on-quick-image-build branch July 12, 2025 09:28
github-actions bot pushed a commit that referenced this pull request Jul 12, 2025
…inutes (#53227)

The #53212 changed the quick-image-build check to only run on
canary build, but this was not the intention - and the image started
to fail because of timeout minutes were too short after we added
python building from sources.

This PR fixes it "properly" - changes timeout minutes to be slightly
longer than the timeout (900 seconds) we specify in build command
and brings back building the image on regular PRs.
(cherry picked from commit 5579edd)

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions
Copy link

Backport successfully created: v3-0-test

Status Branch Result
v3-0-test PR Link

potiuk added a commit that referenced this pull request Jul 12, 2025
…inutes (#53227) (#53230)

The #53212 changed the quick-image-build check to only run on
canary build, but this was not the intention - and the image started
to fail because of timeout minutes were too short after we added
python building from sources.

This PR fixes it "properly" - changes timeout minutes to be slightly
longer than the timeout (900 seconds) we specify in build command
and brings back building the image on regular PRs.
(cherry picked from commit 5579edd)

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
stephen-bracken pushed a commit to stephen-bracken/airflow that referenced this pull request Jul 15, 2025
…he#53227)

The apache#53212 changed the quick-image-build check to only run on
canary build, but this was not the intention - and the image started
to fail because of timeout minutes were too short after we added
python building from sources.

This PR fixes it "properly" - changes timeout minutes to be slightly
longer than the timeout (900 seconds) we specify in build command
and brings back building the image on regular PRs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants