Skip to content

Commit 55dc71d

Browse files
potiukCloud Composer Team
authored and
Cloud Composer Team
committed
fIx isort problems introduced by recent isort release (#28434)
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: PyCQA/isort#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. GitOrigin-RevId: f115b207bc844c10569b2df6fc9acfa32a3c7f41
1 parent 1e46f29 commit 55dc71d

16 files changed

+95
-31
lines changed

.pre-commit-config.yaml

+15-13
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,21 @@ repos:
146146
- --fuzzy-match-generates-todo
147147
files: >
148148
\.cfg$|\.conf$|\.ini$|\.ldif$|\.properties$|\.readthedocs$|\.service$|\.tf$|Dockerfile.*$
149+
- repo: local
150+
hooks:
151+
- id: update-common-sql-api-stubs
152+
name: Check and update common.sql API stubs
153+
entry: ./scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py
154+
language: python
155+
files: ^scripts/ci/pre_commit/pre_commit_update_common_sql_api\.py|^airflow/providers/common/sql/.*\.pyi?$
156+
additional_dependencies: ['rich>=12.4.4', 'mypy==0.971', 'black==22.3.0', 'jinja2']
157+
pass_filenames: false
158+
require_serial: true
159+
- repo: https://github.com/PyCQA/isort
160+
rev: 5.11.2
161+
hooks:
162+
- id: isort
163+
name: Run isort to sort imports in Python files
149164
# Keep version of black in sync wit blacken-docs, pre-commit-hook-names, update-common-sql-api-stubs
150165
- repo: https://github.com/psf/black
151166
rev: 22.3.0
@@ -233,11 +248,6 @@ repos:
233248
entry: yamllint -c yamllint-config.yml --strict
234249
types: [yaml]
235250
exclude: ^.*init_git_sync\.template\.yaml$|^.*airflow\.template\.yaml$|^chart/(?:templates|files)/.*\.yaml$|openapi/.*\.yaml$|^\.pre-commit-config\.yaml$|^airflow/_vendor/
236-
- repo: https://github.com/PyCQA/isort
237-
rev: 5.10.1
238-
hooks:
239-
- id: isort
240-
name: Run isort to sort imports in Python files
241251
- repo: https://github.com/pycqa/pydocstyle
242252
rev: 6.1.1
243253
hooks:
@@ -396,14 +406,6 @@ repos:
396406
language: python
397407
files: ^docs
398408
pass_filenames: false
399-
- id: update-common-sql-api-stubs
400-
name: Check and update common.sql API stubs
401-
entry: ./scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py
402-
language: python
403-
files: ^scripts/ci/pre_commit/pre_commit_update_common_sql_api\.py|^airflow/providers/common/sql/.*\.pyi?$
404-
additional_dependencies: ['rich>=12.4.4', 'mypy==0.971', 'black==22.3.0']
405-
pass_filenames: false
406-
require_serial: true
407409
- id: check-pydevd-left-in-code
408410
language: pygrep
409411
name: Check for pydevd debug statements accidentally left

airflow/providers/common/sql/hooks/sql.pyi

+4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
#
2828
# You can read more in the README_API.md file
2929
#
30+
"""
31+
Definition of the public interface for airflow.providers.common.sql.hooks.sql
32+
isort:skip_file
33+
"""
3034
from _typeshed import Incomplete
3135
from airflow.hooks.dbapi import DbApiHook as BaseForDbApiHook
3236
from typing import Any, Callable, Iterable, Mapping, Sequence

airflow/providers/common/sql/operators/sql.pyi

+4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
#
2828
# You can read more in the README_API.md file
2929
#
30+
"""
31+
Definition of the public interface for airflow.providers.common.sql.operators.sql
32+
isort:skip_file
33+
"""
3034
from _typeshed import Incomplete
3135
from airflow.models import BaseOperator, SkipMixin
3236
from airflow.providers.common.sql.hooks.sql import DbApiHook

airflow/providers/common/sql/sensors/sql.pyi

+4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
#
2828
# You can read more in the README_API.md file
2929
#
30+
"""
31+
Definition of the public interface for airflow.providers.common.sql.sensors.sql
32+
isort:skip_file
33+
"""
3034
from _typeshed import Incomplete
3135
from airflow.sensors.base import BaseSensorOperator
3236
from typing import Any, Sequence

docker_tests/test_docker_compose_quick_start.py

+3
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,13 @@
2828

2929
import requests
3030

31+
# isort:off (needed to workaround isort bug)
3132
from docker_tests.command_utils import run_command
3233
from docker_tests.constants import SOURCE_ROOT
3334
from docker_tests.docker_tests_utils import docker_image
3435

36+
# isort:on (needed to workaround isort bug)
37+
3538
AIRFLOW_WWW_USER_USERNAME = os.environ.get("_AIRFLOW_WWW_USER_USERNAME", "airflow")
3639
AIRFLOW_WWW_USER_PASSWORD = os.environ.get("_AIRFLOW_WWW_USER_PASSWORD", "airflow")
3740
DAG_ID = "example_bash_operator"

docker_tests/test_examples_of_prod_image_building.py

+3
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@
2525
import pytest
2626
import requests
2727

28+
# isort:off (needed to workaround isort bug)
2829
from docker_tests.command_utils import run_command
2930
from docker_tests.constants import SOURCE_ROOT
3031

32+
# isort:on (needed to workaround isort bug)
33+
3134
DOCKER_EXAMPLES_DIR = SOURCE_ROOT / "docs" / "docker-stack" / "docker-examples"
3235

3336

docker_tests/test_prod_image.py

+3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import pytest
2626

27+
# isort:off (needed to workaround isort bug)
2728
from docker_tests.command_utils import run_command
2829
from docker_tests.constants import SOURCE_ROOT
2930
from docker_tests.docker_tests_utils import (
@@ -32,6 +33,8 @@
3233
run_bash_in_docker,
3334
run_python_in_docker,
3435
)
36+
37+
# isort:on (needed to workaround isort bug)
3538
from setup import PREINSTALLED_PROVIDERS
3639

3740
INSTALLED_PROVIDER_PATH = SOURCE_ROOT / "scripts" / "ci" / "installed_providers.txt"

docs/build_docs.py

+8-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@
1515
# KIND, either express or implied. See the License for the
1616
# specific language governing permissions and limitations
1717
# under the License.
18+
"""
19+
Builds documentation and runs spell checking
20+
21+
# isort:skip_file (needed to workaround isort bug)
22+
"""
1823
from __future__ import annotations
1924

2025
import argparse
@@ -25,9 +30,6 @@
2530
from itertools import filterfalse, tee
2631
from typing import Callable, Iterable, NamedTuple, TypeVar
2732

28-
from rich.console import Console
29-
from tabulate import tabulate
30-
3133
from docs.exts.docs_build import dev_index_generator, lint_checks
3234
from docs.exts.docs_build.code_utils import CONSOLE_WIDTH, PROVIDER_INIT_FILE
3335
from docs.exts.docs_build.docs_builder import DOCS_DIR, AirflowDocsBuilder, get_available_packages
@@ -37,6 +39,9 @@
3739
from docs.exts.docs_build.package_filter import process_package_filters
3840
from docs.exts.docs_build.spelling_checks import SpellingError, display_spelling_error_summary
3941

42+
from rich.console import Console
43+
from tabulate import tabulate
44+
4045
TEXT_RED = "\033[31m"
4146
TEXT_RESET = "\033[0m"
4247

docs/exts/docs_build/dev_index_generator.py

+3
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,11 @@
2323

2424
import jinja2
2525

26+
# isort:off (needed to workaround isort bug)
2627
from docs.exts.provider_yaml_utils import load_package_data
2728

29+
# isort:on (needed to workaround isort bug)
30+
2831
CURRENT_DIR = os.path.abspath(os.path.dirname(__file__))
2932
DOCS_DIR = os.path.abspath(os.path.join(CURRENT_DIR, os.pardir, os.pardir))
3033
BUILD_DIR = os.path.abspath(os.path.join(DOCS_DIR, "_build"))

docs/exts/docs_build/errors.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
from rich.console import Console
2424

2525
from airflow.utils.code_utils import prepare_code_snippet
26-
from docs.exts.docs_build.code_utils import CONSOLE_WIDTH
26+
27+
from docs.exts.docs_build.code_utils import CONSOLE_WIDTH # isort:skip (needed to workaround isort bug)
28+
2729

2830
CURRENT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__)))
2931
DOCS_DIR = os.path.abspath(os.path.join(CURRENT_DIR, os.pardir, os.pardir))

docs/publish_docs.py

+3
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@
2121
import argparse
2222
import os
2323

24+
# isort:off (needed to workaround isort bug)
2425
from exts.docs_build.docs_builder import AirflowDocsBuilder
2526
from exts.docs_build.package_filter import process_package_filters
2627
from exts.provider_yaml_utils import load_package_data
2728

29+
# isort:on (needed to workaround isort bug)
30+
2831
AIRFLOW_SITE_DIR = os.environ.get("AIRFLOW_SITE_DIRECTORY")
2932

3033

kubernetes_tests/test_kubernetes_executor.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
import pytest
2222

23-
from kubernetes_tests.test_base import EXECUTOR, TestBase
23+
from kubernetes_tests.test_base import EXECUTOR, TestBase # isort:skip (needed to workaround isort bug)
2424

2525

2626
@pytest.mark.skipif(EXECUTOR != "KubernetesExecutor", reason="Only runs on KubernetesExecutor")

kubernetes_tests/test_other_executors.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
import pytest
2222

23-
from kubernetes_tests.test_base import EXECUTOR, TestBase
23+
from kubernetes_tests.test_base import EXECUTOR, TestBase # isort:skip (needed to workaround isort bug)
2424

2525

2626
# These tests are here because only KubernetesExecutor can run the tests in

pyproject.toml

+1
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ known_first_party = ["airflow", "airflow_breeze", "docker_tests", "docs", "kuber
3535
# The test_python.py is needed because adding __future__.annotations breaks runtime checks that are
3636
# needed for the test to work
3737
skip = ["build", ".tox", "venv", "tests/decorators/test_python.py"]
38+
lines_between_types = 0
3839
skip_glob = ["*.pyi"]
3940
profile = "black"

scripts/ci/pre_commit/pre_commit_update_common_sql_api_stubs.py

+35-11
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import os
2222
import shutil
2323
import subprocess
24+
25+
import jinja2
2426
import sys
2527
import textwrap
2628
from functools import lru_cache
@@ -139,18 +141,22 @@ def post_process_historically_publicised_methods(stub_file_path: Path, line: str
139141
new_lines.append(line)
140142

141143

142-
def post_process_generated_stub_file(stub_file_path: Path, lines: list[str], patch_historical_methods=False):
144+
def post_process_generated_stub_file(
145+
module_name: str, stub_file_path: Path, lines: list[str], patch_historical_methods=False
146+
):
143147
"""
144148
Post process the stub file - add the preamble and optionally patch historical methods.
145149
Adding preamble always, makes sure that we can update the preamble and have it automatically updated
146150
in generated files even if no API specification changes.
147151
152+
:param module_name: name of the module of the file
148153
:param stub_file_path: path of the stub fil
149154
:param lines: lines that were read from the file (with stripped comments)
150155
:param patch_historical_methods: whether we should patch historical methods
151156
:return: resulting lines of the file after post-processing
152157
"""
153-
new_lines = PREAMBLE.splitlines()
158+
template = jinja2.Template(PREAMBLE)
159+
new_lines = template.render(module_name=module_name).splitlines()
154160
for line in lines:
155161
if patch_historical_methods:
156162
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
159165
return new_lines
160166

161167

162-
def read_pyi_file_content(pyi_file_path: Path, patch_historical_methods=False) -> list[str] | None:
168+
def read_pyi_file_content(
169+
module_name: str, pyi_file_path: Path, patch_historical_methods=False
170+
) -> list[str] | None:
163171
"""
164172
Reads stub file content with post-processing and optionally patching historical methods. The comments
165-
are stripped and preamble is always added. It makes sure that we can update the preamble
166-
and have it automatically updated in generated files even if no API specification changes.
173+
and initial javadoc are stripped and preamble is always added. It makes sure that we can update
174+
the preamble and have it automatically updated in generated files even if no API specification changes.
167175
168176
If None is returned, the file should be deleted.
169177
170-
:param pyi_file_path:
171-
:param patch_historical_methods:
178+
:param module_name: name of the module in question
179+
:param pyi_file_path: the path of the file to read
180+
:param patch_historical_methods: whether the historical methods should be patched
172181
:return: list of lines of post-processed content or None if the file should be deleted.
173182
"""
174-
lines = [
183+
lines_no_comments = [
175184
line
176185
for line in pyi_file_path.read_text(encoding="utf-8").splitlines()
177186
if line.strip() and not line.strip().startswith("#")
178187
]
188+
remove_docstring = False
189+
lines = []
190+
for line in lines_no_comments:
191+
if line.strip().startswith('"""'):
192+
remove_docstring = not remove_docstring
193+
continue
194+
if not remove_docstring:
195+
lines.append(line)
179196
if (pyi_file_path.name == "__init__.pyi") and lines == []:
180197
console.print(f"[yellow]Skip {pyi_file_path} as it is an empty stub for __init__.py file")
181198
return None
182199
return post_process_generated_stub_file(
183-
pyi_file_path, lines, patch_historical_methods=patch_historical_methods
200+
module_name, pyi_file_path, lines, patch_historical_methods=patch_historical_methods
184201
)
185202

186203

@@ -194,7 +211,10 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple
194211
_removals, _additions = 0, 0
195212
rel_path = generated_stub_path.relative_to(OUT_DIR)
196213
target_path = PROVIDERS_ROOT / rel_path
197-
generated_pyi_content = read_pyi_file_content(generated_stub_path, patch_historical_methods=True)
214+
module_name = "airflow.providers." + os.fspath(rel_path.with_suffix("")).replace(os.path.sep, ".")
215+
generated_pyi_content = read_pyi_file_content(
216+
module_name, generated_stub_path, patch_historical_methods=True
217+
)
198218
if generated_pyi_content is None:
199219
os.unlink(generated_stub_path)
200220
if target_path.exists():
@@ -217,7 +237,7 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple
217237
console.print(f"[yellow]New file {target_path} has been missing. Treated as addition.")
218238
target_path.write_text("\n".join(generated_pyi_content), encoding="utf-8")
219239
return 0, 1
220-
target_pyi_content = read_pyi_file_content(target_path, patch_historical_methods=False)
240+
target_pyi_content = read_pyi_file_content(module_name, target_path, patch_historical_methods=False)
221241
if target_pyi_content is None:
222242
target_pyi_content = []
223243
if generated_pyi_content != target_pyi_content:
@@ -288,6 +308,10 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple
288308
#
289309
# You can read more in the README_API.md file
290310
#
311+
\"\"\"
312+
Definition of the public interface for {{ module_name }}
313+
isort:skip_file
314+
\"\"\"
291315
"""
292316

293317

setup.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,10 @@ def write_version(filename: str = str(AIRFLOW_SOURCES_ROOT / "airflow" / "git_ve
377377
"flaky",
378378
"gitpython",
379379
"ipdb",
380-
"isort",
380+
# make sure that we are using stable sorting order from 5.* version (some changes were introduced
381+
# in 5.11.3. Black is not compatible yet, so we need to limit isort
382+
# we can remove the limit when black and isort agree on the order
383+
"isort==5.11.2",
381384
"jira",
382385
"jsondiff",
383386
"mongomock",

0 commit comments

Comments
 (0)