From a027992bf8c1fd3f10627de164c51b5d01d95b3c Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 17 Dec 2022 22:41:10 +0100 Subject: [PATCH] fIx isort problems introduced by recent isort release The recent isort changed their mind on sorting the imports. This change follows the change and bumps isort to latest released version (isort has no install_requires on its own so bumping min version has no effect on other dependencies) This change adds a number of isort:skip_file, isort:off, isort:skips in order to handle a very annoying bug in isort, that no matter how much you try, it sometimes treat "known first party" packages differently - depending on how many files it processes at a time. We should be able to restore it after this bug is fixed: https://github.com/PyCQA/isort/issues/2045 This change also updates the common.sql API to skip them from isort for the very same reason (depending on how many files are modified, the isort order might change. --- .pre-commit-config.yaml | 28 +++++------ airflow/providers/common/sql/hooks/sql.pyi | 4 ++ .../providers/common/sql/operators/sql.pyi | 4 ++ airflow/providers/common/sql/sensors/sql.pyi | 4 ++ .../test_docker_compose_quick_start.py | 3 ++ .../test_examples_of_prod_image_building.py | 3 ++ docker_tests/test_prod_image.py | 3 ++ docs/build_docs.py | 11 +++-- docs/exts/docs_build/dev_index_generator.py | 3 ++ docs/exts/docs_build/errors.py | 4 +- docs/publish_docs.py | 3 ++ kubernetes_tests/test_kubernetes_executor.py | 2 +- kubernetes_tests/test_other_executors.py | 2 +- pyproject.toml | 1 + .../pre_commit_update_common_sql_api_stubs.py | 46 ++++++++++++++----- setup.py | 5 +- 16 files changed, 95 insertions(+), 31 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7506249602b3d..8b3f4c732de8e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -146,6 +146,21 @@ repos: - --fuzzy-match-generates-todo files: > \.cfg$|\.conf$|\.ini$|\.ldif$|\.properties$|\.readthedocs$|\.service$|\.tf$|Dockerfile.*$ + - repo: local + hooks: + - id: update-common-sql-api-stubs + name: Check and update common.sql API stubs + entry: ./scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py + language: python + files: ^scripts/ci/pre_commit/pre_commit_update_common_sql_api\.py|^airflow/providers/common/sql/.*\.pyi?$ + additional_dependencies: ['rich>=12.4.4', 'mypy==0.971', 'black==22.3.0', 'jinja2'] + pass_filenames: false + require_serial: true + - repo: https://github.com/PyCQA/isort + rev: 5.11.2 + hooks: + - id: isort + name: Run isort to sort imports in Python files # Keep version of black in sync wit blacken-docs, pre-commit-hook-names, update-common-sql-api-stubs - repo: https://github.com/psf/black rev: 22.3.0 @@ -233,11 +248,6 @@ repos: entry: yamllint -c yamllint-config.yml --strict types: [yaml] exclude: ^.*init_git_sync\.template\.yaml$|^.*airflow\.template\.yaml$|^chart/(?:templates|files)/.*\.yaml$|openapi/.*\.yaml$|^\.pre-commit-config\.yaml$|^airflow/_vendor/ - - repo: https://github.com/PyCQA/isort - rev: 5.10.1 - hooks: - - id: isort - name: Run isort to sort imports in Python files - repo: https://github.com/pycqa/pydocstyle rev: 6.1.1 hooks: @@ -396,14 +406,6 @@ repos: language: python files: ^docs pass_filenames: false - - id: update-common-sql-api-stubs - name: Check and update common.sql API stubs - entry: ./scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py - language: python - files: ^scripts/ci/pre_commit/pre_commit_update_common_sql_api\.py|^airflow/providers/common/sql/.*\.pyi?$ - additional_dependencies: ['rich>=12.4.4', 'mypy==0.971', 'black==22.3.0'] - pass_filenames: false - require_serial: true - id: check-pydevd-left-in-code language: pygrep name: Check for pydevd debug statements accidentally left diff --git a/airflow/providers/common/sql/hooks/sql.pyi b/airflow/providers/common/sql/hooks/sql.pyi index 718f09da7fd3d..30d8eef488d9b 100644 --- a/airflow/providers/common/sql/hooks/sql.pyi +++ b/airflow/providers/common/sql/hooks/sql.pyi @@ -27,6 +27,10 @@ # # You can read more in the README_API.md file # +""" +Definition of the public interface for airflow.providers.common.sql.hooks.sql +isort:skip_file +""" from _typeshed import Incomplete from airflow.hooks.dbapi import DbApiHook as BaseForDbApiHook from typing import Any, Callable, Iterable, Mapping, Sequence diff --git a/airflow/providers/common/sql/operators/sql.pyi b/airflow/providers/common/sql/operators/sql.pyi index 70e24ce2409eb..5e95dd8c5771c 100644 --- a/airflow/providers/common/sql/operators/sql.pyi +++ b/airflow/providers/common/sql/operators/sql.pyi @@ -27,6 +27,10 @@ # # You can read more in the README_API.md file # +""" +Definition of the public interface for airflow.providers.common.sql.operators.sql +isort:skip_file +""" from _typeshed import Incomplete from airflow.models import BaseOperator, SkipMixin from airflow.providers.common.sql.hooks.sql import DbApiHook diff --git a/airflow/providers/common/sql/sensors/sql.pyi b/airflow/providers/common/sql/sensors/sql.pyi index d97370c7e04ee..ed88c4b8109fe 100644 --- a/airflow/providers/common/sql/sensors/sql.pyi +++ b/airflow/providers/common/sql/sensors/sql.pyi @@ -27,6 +27,10 @@ # # You can read more in the README_API.md file # +""" +Definition of the public interface for airflow.providers.common.sql.sensors.sql +isort:skip_file +""" from _typeshed import Incomplete from airflow.sensors.base import BaseSensorOperator from typing import Any, Sequence diff --git a/docker_tests/test_docker_compose_quick_start.py b/docker_tests/test_docker_compose_quick_start.py index 6f25f625788d4..4754aac3299ab 100644 --- a/docker_tests/test_docker_compose_quick_start.py +++ b/docker_tests/test_docker_compose_quick_start.py @@ -28,10 +28,13 @@ import requests +# isort:off (needed to workaround isort bug) from docker_tests.command_utils import run_command from docker_tests.constants import SOURCE_ROOT from docker_tests.docker_tests_utils import docker_image +# isort:on (needed to workaround isort bug) + AIRFLOW_WWW_USER_USERNAME = os.environ.get("_AIRFLOW_WWW_USER_USERNAME", "airflow") AIRFLOW_WWW_USER_PASSWORD = os.environ.get("_AIRFLOW_WWW_USER_PASSWORD", "airflow") DAG_ID = "example_bash_operator" diff --git a/docker_tests/test_examples_of_prod_image_building.py b/docker_tests/test_examples_of_prod_image_building.py index 978473e64e4ec..858e4c1e8fc9c 100644 --- a/docker_tests/test_examples_of_prod_image_building.py +++ b/docker_tests/test_examples_of_prod_image_building.py @@ -25,9 +25,12 @@ import pytest import requests +# isort:off (needed to workaround isort bug) from docker_tests.command_utils import run_command from docker_tests.constants import SOURCE_ROOT +# isort:on (needed to workaround isort bug) + DOCKER_EXAMPLES_DIR = SOURCE_ROOT / "docs" / "docker-stack" / "docker-examples" diff --git a/docker_tests/test_prod_image.py b/docker_tests/test_prod_image.py index f1d7fbd0180a9..8ad2d89a9f567 100644 --- a/docker_tests/test_prod_image.py +++ b/docker_tests/test_prod_image.py @@ -24,6 +24,7 @@ import pytest +# isort:off (needed to workaround isort bug) from docker_tests.command_utils import run_command from docker_tests.constants import SOURCE_ROOT from docker_tests.docker_tests_utils import ( @@ -32,6 +33,8 @@ run_bash_in_docker, run_python_in_docker, ) + +# isort:on (needed to workaround isort bug) from setup import PREINSTALLED_PROVIDERS INSTALLED_PROVIDER_PATH = SOURCE_ROOT / "scripts" / "ci" / "installed_providers.txt" diff --git a/docs/build_docs.py b/docs/build_docs.py index d1fb06ccacad1..273858be7636a 100755 --- a/docs/build_docs.py +++ b/docs/build_docs.py @@ -15,6 +15,11 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +""" +Builds documentation and runs spell checking + +# isort:skip_file (needed to workaround isort bug) +""" from __future__ import annotations import argparse @@ -25,9 +30,6 @@ from itertools import filterfalse, tee from typing import Callable, Iterable, NamedTuple, TypeVar -from rich.console import Console -from tabulate import tabulate - from docs.exts.docs_build import dev_index_generator, lint_checks from docs.exts.docs_build.code_utils import CONSOLE_WIDTH, PROVIDER_INIT_FILE from docs.exts.docs_build.docs_builder import DOCS_DIR, AirflowDocsBuilder, get_available_packages @@ -37,6 +39,9 @@ from docs.exts.docs_build.package_filter import process_package_filters from docs.exts.docs_build.spelling_checks import SpellingError, display_spelling_error_summary +from rich.console import Console +from tabulate import tabulate + TEXT_RED = "\033[31m" TEXT_RESET = "\033[0m" diff --git a/docs/exts/docs_build/dev_index_generator.py b/docs/exts/docs_build/dev_index_generator.py index f423ed20f7703..0aee91b8f4a94 100644 --- a/docs/exts/docs_build/dev_index_generator.py +++ b/docs/exts/docs_build/dev_index_generator.py @@ -23,8 +23,11 @@ import jinja2 +# isort:off (needed to workaround isort bug) from docs.exts.provider_yaml_utils import load_package_data +# isort:on (needed to workaround isort bug) + CURRENT_DIR = os.path.abspath(os.path.dirname(__file__)) DOCS_DIR = os.path.abspath(os.path.join(CURRENT_DIR, os.pardir, os.pardir)) BUILD_DIR = os.path.abspath(os.path.join(DOCS_DIR, "_build")) diff --git a/docs/exts/docs_build/errors.py b/docs/exts/docs_build/errors.py index 187b89c15aaf2..321379896ca87 100644 --- a/docs/exts/docs_build/errors.py +++ b/docs/exts/docs_build/errors.py @@ -23,7 +23,9 @@ from rich.console import Console from airflow.utils.code_utils import prepare_code_snippet -from docs.exts.docs_build.code_utils import CONSOLE_WIDTH + +from docs.exts.docs_build.code_utils import CONSOLE_WIDTH # isort:skip (needed to workaround isort bug) + CURRENT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__))) DOCS_DIR = os.path.abspath(os.path.join(CURRENT_DIR, os.pardir, os.pardir)) diff --git a/docs/publish_docs.py b/docs/publish_docs.py index bf337995043a9..9a17aa29ef48e 100755 --- a/docs/publish_docs.py +++ b/docs/publish_docs.py @@ -21,10 +21,13 @@ import argparse import os +# isort:off (needed to workaround isort bug) from exts.docs_build.docs_builder import AirflowDocsBuilder from exts.docs_build.package_filter import process_package_filters from exts.provider_yaml_utils import load_package_data +# isort:on (needed to workaround isort bug) + AIRFLOW_SITE_DIR = os.environ.get("AIRFLOW_SITE_DIRECTORY") diff --git a/kubernetes_tests/test_kubernetes_executor.py b/kubernetes_tests/test_kubernetes_executor.py index 7f40a976103fc..95afb25590f44 100644 --- a/kubernetes_tests/test_kubernetes_executor.py +++ b/kubernetes_tests/test_kubernetes_executor.py @@ -20,7 +20,7 @@ import pytest -from kubernetes_tests.test_base import EXECUTOR, TestBase +from kubernetes_tests.test_base import EXECUTOR, TestBase # isort:skip (needed to workaround isort bug) @pytest.mark.skipif(EXECUTOR != "KubernetesExecutor", reason="Only runs on KubernetesExecutor") diff --git a/kubernetes_tests/test_other_executors.py b/kubernetes_tests/test_other_executors.py index f517fdc82fd5c..42a8258871a2c 100644 --- a/kubernetes_tests/test_other_executors.py +++ b/kubernetes_tests/test_other_executors.py @@ -20,7 +20,7 @@ import pytest -from kubernetes_tests.test_base import EXECUTOR, TestBase +from kubernetes_tests.test_base import EXECUTOR, TestBase # isort:skip (needed to workaround isort bug) # These tests are here because only KubernetesExecutor can run the tests in diff --git a/pyproject.toml b/pyproject.toml index 39cfef27e47d4..a40b7d9179902 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,5 +35,6 @@ known_first_party = ["airflow", "airflow_breeze", "docker_tests", "docs", "kuber # The test_python.py is needed because adding __future__.annotations breaks runtime checks that are # needed for the test to work skip = ["build", ".tox", "venv", "tests/decorators/test_python.py"] +lines_between_types = 0 skip_glob = ["*.pyi"] profile = "black" diff --git a/scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py b/scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py index 98f1bd556991a..5a85b84a0c844 100755 --- a/scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py +++ b/scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py @@ -21,6 +21,8 @@ import os import shutil import subprocess + +import jinja2 import sys import textwrap from functools import lru_cache @@ -139,18 +141,22 @@ def post_process_historically_publicised_methods(stub_file_path: Path, line: str new_lines.append(line) -def post_process_generated_stub_file(stub_file_path: Path, lines: list[str], patch_historical_methods=False): +def post_process_generated_stub_file( + module_name: str, stub_file_path: Path, lines: list[str], patch_historical_methods=False +): """ Post process the stub file - add the preamble and optionally patch historical methods. Adding preamble always, makes sure that we can update the preamble and have it automatically updated in generated files even if no API specification changes. + :param module_name: name of the module of the file :param stub_file_path: path of the stub fil :param lines: lines that were read from the file (with stripped comments) :param patch_historical_methods: whether we should patch historical methods :return: resulting lines of the file after post-processing """ - new_lines = PREAMBLE.splitlines() + template = jinja2.Template(PREAMBLE) + new_lines = template.render(module_name=module_name).splitlines() for line in lines: if patch_historical_methods: post_process_historically_publicised_methods(stub_file_path, line, new_lines) @@ -159,28 +165,39 @@ def post_process_generated_stub_file(stub_file_path: Path, lines: list[str], pat return new_lines -def read_pyi_file_content(pyi_file_path: Path, patch_historical_methods=False) -> list[str] | None: +def read_pyi_file_content( + module_name: str, pyi_file_path: Path, patch_historical_methods=False +) -> list[str] | None: """ Reads stub file content with post-processing and optionally patching historical methods. The comments - are stripped and preamble is always added. It makes sure that we can update the preamble - and have it automatically updated in generated files even if no API specification changes. + and initial javadoc are stripped and preamble is always added. It makes sure that we can update + the preamble and have it automatically updated in generated files even if no API specification changes. If None is returned, the file should be deleted. - :param pyi_file_path: - :param patch_historical_methods: + :param module_name: name of the module in question + :param pyi_file_path: the path of the file to read + :param patch_historical_methods: whether the historical methods should be patched :return: list of lines of post-processed content or None if the file should be deleted. """ - lines = [ + lines_no_comments = [ line for line in pyi_file_path.read_text(encoding="utf-8").splitlines() if line.strip() and not line.strip().startswith("#") ] + remove_docstring = False + lines = [] + for line in lines_no_comments: + if line.strip().startswith('"""'): + remove_docstring = not remove_docstring + continue + if not remove_docstring: + lines.append(line) if (pyi_file_path.name == "__init__.pyi") and lines == []: console.print(f"[yellow]Skip {pyi_file_path} as it is an empty stub for __init__.py file") return None return post_process_generated_stub_file( - pyi_file_path, lines, patch_historical_methods=patch_historical_methods + module_name, pyi_file_path, lines, patch_historical_methods=patch_historical_methods ) @@ -194,7 +211,10 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple _removals, _additions = 0, 0 rel_path = generated_stub_path.relative_to(OUT_DIR) target_path = PROVIDERS_ROOT / rel_path - generated_pyi_content = read_pyi_file_content(generated_stub_path, patch_historical_methods=True) + module_name = "airflow.providers." + os.fspath(rel_path.with_suffix("")).replace(os.path.sep, ".") + generated_pyi_content = read_pyi_file_content( + module_name, generated_stub_path, patch_historical_methods=True + ) if generated_pyi_content is None: os.unlink(generated_stub_path) if target_path.exists(): @@ -217,7 +237,7 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple console.print(f"[yellow]New file {target_path} has been missing. Treated as addition.") target_path.write_text("\n".join(generated_pyi_content), encoding="utf-8") return 0, 1 - target_pyi_content = read_pyi_file_content(target_path, patch_historical_methods=False) + target_pyi_content = read_pyi_file_content(module_name, target_path, patch_historical_methods=False) if target_pyi_content is None: target_pyi_content = [] if generated_pyi_content != target_pyi_content: @@ -288,6 +308,10 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple # # You can read more in the README_API.md file # +\"\"\" +Definition of the public interface for {{ module_name }} +isort:skip_file +\"\"\" """ diff --git a/setup.py b/setup.py index 876c73b36e48e..e2ec44fef1cfe 100644 --- a/setup.py +++ b/setup.py @@ -377,7 +377,10 @@ def write_version(filename: str = str(AIRFLOW_SOURCES_ROOT / "airflow" / "git_ve "flaky", "gitpython", "ipdb", - "isort", + # make sure that we are using stable sorting order from 5.* version (some changes were introduced + # in 5.11.3. Black is not compatible yet, so we need to limit isort + # we can remove the limit when black and isort agree on the order + "isort==5.11.2", "jira", "jsondiff", "mongomock",