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 2023/11/27 02:05:02 UTC
[PR] Convert mypy pre-commits to use breeze shell to run them [airflow]
potiuk opened a new pull request, #35877:
URL: https://github.com/apache/airflow/pull/35877
The Mypy static checks now use breze shell to execute, also they
force color so the errors are seen with coloured output even when
they are run as part of git hook.
Scripts folder needs to be added in its entirety in order to be
able to run the mypy check without mounting the volumes to
running container - but relying on the code already in the image.
<!--
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.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 [newsfragments](https://github.com/apache/airflow/tree/main/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] Convert mypy pre-commits to use breeze shell to run them [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35877:
URL: https://github.com/apache/airflow/pull/35877#issuecomment-1827020375
Based on #35875
--
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] Convert mypy pre-commits to use breeze shell to run them [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #35877:
URL: https://github.com/apache/airflow/pull/35877
--
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] Convert mypy pre-commits to use breeze shell to run them [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #35877:
URL: https://github.com/apache/airflow/pull/35877#discussion_r1405561342
##########
scripts/ci/pre_commit/pre_commit_mypy.py:
##########
@@ -21,72 +21,45 @@
import sys
from pathlib import Path
-if __name__ not in ("__main__", "__mp_main__"):
- raise SystemExit(
- "This file is intended to be executed as an executable program. You cannot use it as a module."
- f"To run this script, run the ./{__file__} command"
- )
-
+sys.path.insert(0, str(Path(__file__).parent.resolve()))
-AIRFLOW_SOURCES = Path(__file__).parents[3].resolve()
-GITHUB_REPOSITORY = os.environ.get("GITHUB_REPOSITORY", "apache/airflow")
-os.environ["SKIP_GROUP_OUTPUT"] = "true"
+from common_precommit_utils import (
+ console,
+ filter_out_providers_on_non_main_branch,
+ initialize_breeze_precommit,
+ run_command_via_breeze_shell,
+)
-if __name__ == "__main__":
- sys.path.insert(0, str(Path(__file__).parent.resolve())) # make sure common_precommit_utils is imported
+initialize_breeze_precommit(__name__, __file__)
- os.environ["SKIP_BREEZE_SELF_UPGRADE_CHECK"] = "true"
- from common_precommit_utils import filter_out_providers_on_non_main_branch
-
- sys.path.insert(0, str(AIRFLOW_SOURCES / "dev" / "breeze" / "src"))
- from airflow_breeze.global_constants import DEFAULT_PYTHON_MAJOR_MINOR_VERSION, MOUNT_SELECTED
- from airflow_breeze.params.shell_params import ShellParams
- from airflow_breeze.utils.console import get_console # isort: skip
- from airflow_breeze.utils.docker_command_utils import get_extra_docker_flags # isort: skip
- from airflow_breeze.utils.path_utils import create_mypy_volume_if_needed # isort: skip
- from airflow_breeze.utils.run_utils import (
- get_ci_image_for_pre_commits,
- run_command,
- )
+files_to_test = filter_out_providers_on_non_main_branch(sys.argv[1:])
+if files_to_test == ["--namespace-packages"]:
+ print("No files to tests. Quitting")
+ sys.exit(0)
- shell_params = ShellParams(python=DEFAULT_PYTHON_MAJOR_MINOR_VERSION, backend="none")
-
- files_to_test = filter_out_providers_on_non_main_branch(sys.argv[1:])
- if files_to_test == ["--namespace-packages"]:
- print("No files to tests. Quitting")
- sys.exit(0)
- airflow_image = get_ci_image_for_pre_commits()
- create_mypy_volume_if_needed()
- cmd_result = run_command(
- [
- "docker",
- "run",
- "-t",
- *get_extra_docker_flags(mount_sources=MOUNT_SELECTED, include_mypy_volume=True),
- "-e",
- "SKIP_ENVIRONMENT_INITIALIZATION=true",
- "--pull",
- "never",
- airflow_image,
- "/opt/airflow/scripts/in_container/run_mypy.sh",
- *files_to_test,
- ],
- check=False,
- env=shell_params.env_variables_for_docker_commands,
- )
- if cmd_result.returncode != 0:
- upgrading = os.environ.get("UPGRADE_TO_NEWER_DEPENDENCIES", "false") != "false"
- if upgrading:
- get_console().print(
- "[warning]You are running mypy with the image that has dependencies upgraded automatically."
- )
- flag = " --upgrade-to-newer-dependencies" if upgrading else ""
- get_console().print(
- "[warning]If you see strange stacktraces above, "
- f"run `breeze ci-image build --python 3.8{flag}` and try again. "
- "You can also run `breeze down --cleanup-mypy-cache` to clean up the cache used. "
- "Still sometimes diff heuristic in mypy is behaving abnormal, to double check you can "
- "call `breeze static-checks --type mypy-[dev|core|providers|docs] --all-files` "
- 'and then commit via `git commit --no-verify -m "commit message"`. CI will do a full check.'
+res = run_command_via_breeze_shell(
+ [
+ "/opt/airflow/scripts/in_container/run_mypy.sh",
+ *files_to_test,
+ ],
+ warn_image_upgrade_needed=True,
Review Comment:
cc: @jscheffl -> also added it for MyPy.
--
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