You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2025/08/17 11:51:35 UTC
[PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
potiuk opened a new pull request, #54585:
URL: https://github.com/apache/airflow/pull/54585
The pre-commit is a fantastic tool, and we heavily used it for years, but generally the tool stagnated and is not showing a sign of adapting to our needs. For years we tried to convince pre-commit maintainers that things like autocomplete are necessary - but it met with pretty much resistance (if not hostility) from the maintainer. Also there was no chance for them to accept expectations of bigger projects like ours, where we have a huge monorepo and not only multiple needs but also different parts of the repo needing different language support (golang, typescript soon) - and apparenty the maintainer of pre-commit does not think monorepo is a good thing at all. Similarly - they did not recognize the raise of `uv` and the only way to use `uv` with pre-commit is to patch it by installing `pre-comit-uv` that essentialy patches pre-commit with uv support. This is not really sustainable and the tool lags behind many of our needs.
Luckily - we have new project in town - prek - which rewrites pre-commit that is 100% compatible (now), 10x faster (because rust), uses `uv` natively, supports auto-complete already and they have very friendly maintainer who is not only supporting us but also very happily works on improving `prek` to close all the gaps, and plans to implement (with our support of course and cooperation) monorepo support - that will allow us to modularise our pre-commits.
This PR switches our pre-commit support to use prek exclusively:
* breeze static checks command is completely removed
* custom auto-complete code in breeze as well
* instructions are updated to setup prek instead of precommit
* CI is updated to run prek instead of pre-commmit
* documentation for static checks is reviewed and new features that prek enables are added
(cherry picked from commit 3442d811cf693e836a862652c9caf04b7a373af7)
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280864400
##########
Dockerfile.ci:
##########
@@ -584,20 +584,19 @@ function common::install_packaging_tools() {
pip install --root-user-action ignore --disable-pip-version-check "uv==${AIRFLOW_UV_VERSION}"
fi
fi
- if [[ ${AIRFLOW_PRE_COMMIT_VERSION=} == "" ]]; then
+ if [[ ${AIRFLOW_PREK_VERSION=} == "" ]]; then
echo
- echo "${COLOR_BLUE}Installing latest pre-commit with pre-commit-uv uv${COLOR_RESET}"
+ echo "${COLOR_BLUE}Installing latest preh, uv${COLOR_RESET}"
Review Comment:
Yeah. I will update it in main as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280864018
##########
AGENTS.md:
##########
@@ -0,0 +1,47 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ -->
+
+# AGENTS instructions
Review Comment:
Yes. I synchronized with main, and since this file was modified, I decided to bring it in (there were many more "breeze" syncing changes involved in order to make this PR fail - they are mostly "make breeze as close to main as possible" -main differences are python versions and some hooks and distributions missing (arflow-ctl related).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #54585:
URL: https://github.com/apache/airflow/pull/54585#issuecomment-3194335606
@jscheffl -> Was not that bad. I had to sync breeze with latest main (not diffocult) and bring back a few Python 3.9 construncts in that code, but other than that, it was rather painless. Let's see if the CI agrees as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280872894
##########
dev/breeze/doc/05_test_commands.rst:
##########
@@ -317,6 +317,25 @@ through ``breeze testing docker-compose-tests`` command.
The docker-compose tests are in ``docker-tests/`` folder in the main repo.
+Running task-sdk integration tests
+..................................
+
+You can use Breeze to run the task sdk integration tests. Those tests are run using Production image by default
+and the tests are running with the docker-compose we have for task-sdk tests.
+
+.. image:: ./images/output_testing_docker-compose-tests.svg
+ :target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/images/output_testing_task-sdk-integration-tests.svg
+ :width: 100%
+ :alt: Breeze testing task-sdk-integration-tests
+
+You can also iterate over those tests with pytest command but unlike regular unit tests, they need to be run in
+a local venv. You can build the prod image with breeze and that will be used by default if present to run the tests.
+
+You can override the ``DOCKER_IMAGE`` environment variable to point to the image to test using the
+``breeze testing task-sdk-integration-tests`` command.
Review Comment:
These tests are also backported and now this is a backport of the docs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #54585:
URL: https://github.com/apache/airflow/pull/54585
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #54585:
URL: https://github.com/apache/airflow/pull/54585#issuecomment-3194353795
Few breeze unit tests, but should be fixed now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "aritra24 (via GitHub)" <gi...@apache.org>.
aritra24 commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280860765
##########
AGENTS.md:
##########
@@ -0,0 +1,47 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ -->
+
+# AGENTS instructions
Review Comment:
Is this file intentionally added? Seems unrelated to prek. I'm okay with agent instructions being added, but just wanted to confirm.
##########
Dockerfile.ci:
##########
@@ -584,20 +584,19 @@ function common::install_packaging_tools() {
pip install --root-user-action ignore --disable-pip-version-check "uv==${AIRFLOW_UV_VERSION}"
fi
fi
- if [[ ${AIRFLOW_PRE_COMMIT_VERSION=} == "" ]]; then
+ if [[ ${AIRFLOW_PREK_VERSION=} == "" ]]; then
echo
- echo "${COLOR_BLUE}Installing latest pre-commit with pre-commit-uv uv${COLOR_RESET}"
+ echo "${COLOR_BLUE}Installing latest preh, uv${COLOR_RESET}"
Review Comment:
Same
##########
Dockerfile.ci:
##########
@@ -1314,7 +1313,7 @@ ENV DEV_APT_COMMAND=${DEV_APT_COMMAND} \
ARG AIRFLOW_PYTHON_VERSION=v3.10.18
ENV AIRFLOW_PYTHON_VERSION=$AIRFLOW_PYTHON_VERSION
-ENV GOLANG_MAJOR_MINOR_VERSION=1.24.6
+ENV GOLANG_MAJOR_MINOR_VERSION=1.25.0
Review Comment:
Do we want to bump up golang version right now? Not sure what our stance of golang version support is and timeframe for version bumps.
##########
Dockerfile:
##########
@@ -615,20 +615,19 @@ function common::install_packaging_tools() {
pip install --root-user-action ignore --disable-pip-version-check "uv==${AIRFLOW_UV_VERSION}"
fi
fi
- if [[ ${AIRFLOW_PRE_COMMIT_VERSION=} == "" ]]; then
+ if [[ ${AIRFLOW_PREK_VERSION=} == "" ]]; then
echo
- echo "${COLOR_BLUE}Installing latest pre-commit with pre-commit-uv uv${COLOR_RESET}"
+ echo "${COLOR_BLUE}Installing latest preh, uv${COLOR_RESET}"
Review Comment:
Typo
##########
contributing-docs/06_development_environments.rst:
##########
@@ -57,9 +57,9 @@ Limitations:
Breeze container-based solution provides a reproducible environment that is
consistent with other developers.
-- You are **STRONGLY** encouraged to also install and use `pre-commit hooks <08_static_code_checks.rst#pre-commit-hooks>`_
+- You are **STRONGLY** encouraged to also install and use `prek hooks <08_static_code_checks.rst#prekt-hooks>`_
Review Comment:
Is this a typo prekt/prek
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280864678
##########
Dockerfile.ci:
##########
@@ -1314,7 +1313,7 @@ ENV DEV_APT_COMMAND=${DEV_APT_COMMAND} \
ARG AIRFLOW_PYTHON_VERSION=v3.10.18
ENV AIRFLOW_PYTHON_VERSION=$AIRFLOW_PYTHON_VERSION
-ENV GOLANG_MAJOR_MINOR_VERSION=1.24.6
+ENV GOLANG_MAJOR_MINOR_VERSION=1.25.0
Review Comment:
We do not use `golang` in v3-0-test - but those changes are mostly to synchronize "breeze" / "CI" and getting them as close as possible v3-0-test makes things far easier to cherry-pick later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #54585:
URL: https://github.com/apache/airflow/pull/54585#issuecomment-3194362720
Thanks @aritra24 ! addressed all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280876402
##########
dev/breeze/doc/05_test_commands.rst:
##########
@@ -317,6 +317,25 @@ through ``breeze testing docker-compose-tests`` command.
The docker-compose tests are in ``docker-tests/`` folder in the main repo.
+Running task-sdk integration tests
+..................................
+
+You can use Breeze to run the task sdk integration tests. Those tests are run using Production image by default
+and the tests are running with the docker-compose we have for task-sdk tests.
+
+.. image:: ./images/output_testing_docker-compose-tests.svg
+ :target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/images/output_testing_task-sdk-integration-tests.svg
+ :width: 100%
+ :alt: Breeze testing task-sdk-integration-tests
+
+You can also iterate over those tests with pytest command but unlike regular unit tests, they need to be run in
+a local venv. You can build the prod image with breeze and that will be used by default if present to run the tests.
+
+You can override the ``DOCKER_IMAGE`` environment variable to point to the image to test using the
+``breeze testing task-sdk-integration-tests`` command.
Review Comment:
The "tool" is backported - but I have not backported the tests themselves for task-sdk. But making the changes in breeze to be "on-par" with main, allows to do it actually (but I wanted to limit this PR to only synchronize "dev/ci" framework - but not necessarliy tests themselves.
I also don't think it's strictly needed in this case - but it would be great to actually do that. Those could be relatively easy backported - I think, but I will leave it to @amoghrajesh .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [v3-0-test] Switch pre-commit to prek (#54258) [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #54585:
URL: https://github.com/apache/airflow/pull/54585#discussion_r2280876402
##########
dev/breeze/doc/05_test_commands.rst:
##########
@@ -317,6 +317,25 @@ through ``breeze testing docker-compose-tests`` command.
The docker-compose tests are in ``docker-tests/`` folder in the main repo.
+Running task-sdk integration tests
+..................................
+
+You can use Breeze to run the task sdk integration tests. Those tests are run using Production image by default
+and the tests are running with the docker-compose we have for task-sdk tests.
+
+.. image:: ./images/output_testing_docker-compose-tests.svg
+ :target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/images/output_testing_task-sdk-integration-tests.svg
+ :width: 100%
+ :alt: Breeze testing task-sdk-integration-tests
+
+You can also iterate over those tests with pytest command but unlike regular unit tests, they need to be run in
+a local venv. You can build the prod image with breeze and that will be used by default if present to run the tests.
+
+You can override the ``DOCKER_IMAGE`` environment variable to point to the image to test using the
+``breeze testing task-sdk-integration-tests`` command.
Review Comment:
The "tool" is backported - but I have not backported the tests themselves for task-sdk. But making the changes in breeze to be "on-par" with main, allows to do it actually (but I wanted to limit this PR to only synchronize "dev/ci" framework - but not necessarliy tests themselves.
I also don't think it's strictly needed in this case - it would be great to actually do that. Those could be relatively easy backported - I think, but I will leave it to @amoghrajesh .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org