Skip to content

Conversation

@jscheffl
Copy link
Contributor

Following #57181 this is now a batch for all prek hooks that are "just" solely for providers to split out from root:

As prek is supporting monorepo now and go SDK was the front-runner, Airflow-Core is now the next piece that with this PR is proposed to be split-out from global pre-commit hook list into the provider space.

Note: This is the crown and msot probably atm the last thing to move. Careful review please!

@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 Oct 26, 2025
@jscheffl
Copy link
Contributor Author

jscheffl commented Oct 26, 2025

@potiuk ^^^ Careful review please! - sure you were able to read all the text in this lightning speed? Or trusting me and CI? :-D

@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

Trusting you and CI .. I did took a look "briefly" ....

@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

Trusting you and CI .. I did took a look "briefly" ....

I.e. I am checking if you are not doing anything "sneaky" - but I belive CI is way better than me to check for actual errors in this case.

@jscheffl jscheffl force-pushed the feature/extract-pre-commit-for-airflow-core branch from 56fad0f to 24c8e7c Compare October 26, 2025 18:29
@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

node: error while loading shared libraries: libatomic.so.1: cannot open shared object file: No such file or directory

That looks like prek issue with node installing for a sub-folder pre-commit.

@jscheffl
Copy link
Contributor Author

Parking this PR in regards of
j178/prek#973
as it might need a bit triaging why the constrains generation fails. Super-seeded with another PR and "parking" the leftover.

@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

Let me try something .. It's strange it would fail like that but maybe ...

@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

Pushed a possible fixup

@jscheffl
Copy link
Contributor Author

Pushed a possible fixup

Sounds reasonable... I can confirm I can reproduce it locally but ONLY when I start via breeze release-management, locally the prek hook works - also if I clean the cache prior execution. So might be really a missing dependency int he container (only?). But why when not called in root...

@jscheffl jscheffl added full tests needed We need to run full set of tests for this PR to merge all versions If set, the CI build will be forced to use all versions of Python/K8S/DBs labels Oct 26, 2025
@jscheffl
Copy link
Contributor Author

Just a marker before merge:

  • If this is the fix we should test all version prior merge in my view. Just to be on the safe side. And leave a comment for the future.

@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

Nope. Does not work...

@potiuk
Copy link
Member

potiuk commented Oct 26, 2025

libatomic1 is already the newest version (12.2.0-14+deb12u1).
libatomic1 set to manually installed.
0 upgraded, 0 newly installed, 0 to remove an

Libatomic is already installed. It looks like - possibly - somehow the system library path is removed during the installation by prek. I will comment there.

@j178
Copy link

j178 commented Oct 27, 2025

diff --git a/dev/breeze/src/airflow_breeze/commands/release_management_commands.py b/dev/breeze/src/airflow_breeze/commands/release_management_commands.py
index dfad85b2c7..1b0f28a9d2 100644
--- a/dev/breeze/src/airflow_breeze/commands/release_management_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/release_management_commands.py
@@ -272,7 +272,7 @@ PYYAML_VERSION = "6.0.3"
 AIRFLOW_BUILD_DOCKERFILE = f"""
 # syntax=docker/dockerfile:1.4
 FROM python:{DEFAULT_PYTHON_MAJOR_MINOR_VERSION}-slim-{ALLOWED_DEBIAN_VERSIONS[0]}
-RUN apt-get update && apt-get install -y --no-install-recommends git curl
+RUN apt-get update && apt-get install -y --no-install-recommends libatomic1 git curl
 RUN pip install uv=={UV_VERSION}
 RUN --mount=type=cache,id=cache-airflow-build-dockerfile-installation,target=/root/.cache/ \
   uv pip install --system ignore pip=={AIRFLOW_PIP_VERSION} hatch=={HATCH_VERSION} \

Could you try this patch? I think the breeze release-management prepare-airflow-distributions command is using this dockerfile that gets generated on the fly, and the “slim” image doesn’t come with libatomic1.

@jscheffl jscheffl force-pushed the feature/extract-pre-commit-for-airflow-core branch from d14f530 to 4aa3171 Compare October 27, 2025 21:28
@jscheffl
Copy link
Contributor Author

Could you try this patch? I think the breeze release-management prepare-airflow-distributions command is using this dockerfile that gets generated on the fly, and the “slim” image doesn’t come with libatomic1.

Thanks @j178 for the fast feedback! Re-based on the leftovers and cleaned the PR, applied the workaround as proposed. Let's see if CI turns green via this.

@jscheffl
Copy link
Contributor Author

Seems CI failues are unrelated to the PR now and the proposal by @j178 fixes the problem. Unsure why it is working w/o and if defined in the root. @potiuk LGTM?

@jscheffl
Copy link
Contributor Author

closes: j178/prek#973

@potiuk potiuk force-pushed the feature/extract-pre-commit-for-airflow-core branch from 4aa3171 to b449dfc Compare October 28, 2025 00:33
@potiuk
Copy link
Member

potiuk commented Oct 28, 2025

Rebased to see. Indeed - release management commands use their own smaller "airflow-build-dockerfile"

@potiuk potiuk marked this pull request as ready for review October 28, 2025 00:46
@jscheffl jscheffl force-pushed the feature/extract-pre-commit-for-airflow-core branch from b449dfc to 265159d Compare October 28, 2025 20:24
@jscheffl jscheffl force-pushed the feature/extract-pre-commit-for-airflow-core branch from 265159d to 47ed100 Compare October 28, 2025 20:48
@jscheffl
Copy link
Contributor Author

We have no "luck" with this PR but all errors seem unrelated. Same errors on main. Therefore: merging.

THANKS @j178 for the proposed fix!

@jscheffl jscheffl merged commit a6d3e2d into apache:main Oct 28, 2025
170 of 193 checks passed
@github-actions
Copy link

Backport failed to create: v3-1-test. View the failure log Run details

Status Branch Result
v3-1-test Commit Link

You can attempt to backport this manually by running:

cherry_picker a6d3e2d v3-1-test

This should apply the commit to the v3-1-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

@jscheffl
Copy link
Contributor Author

As we also did not backport #57315 would also skip here? Or should we?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

all versions If set, the CI build will be forced to use all versions of Python/K8S/DBs area:dev-tools backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants