diff --git a/providers/.pre-commit-config.yaml b/providers/.pre-commit-config.yaml index 2cf29a4957e68..4926188916f9d 100644 --- a/providers/.pre-commit-config.yaml +++ b/providers/.pre-commit-config.yaml @@ -240,14 +240,6 @@ repos: language: python files: ^.*/src/airflow/providers/.*version_compat.*\.py$ require_serial: true - - id: check-conf-import-in-providers - name: Check conf is imported from compat SDK in providers - entry: ../scripts/ci/prek/check_conf_import_in_providers.py - language: python - types: [python] - files: ^.*/src/airflow/providers/.*\.py$ - exclude: ^.*/common/compat/sdk\.py$ - require_serial: false - id: provider-version-compat name: Check for correct version_compat imports in providers entry: ../scripts/ci/prek/check_provider_version_compat.py diff --git a/scripts/ci/prek/check_conf_import_in_providers.py b/scripts/ci/prek/check_conf_import_in_providers.py deleted file mode 100755 index b2186377a9621..0000000000000 --- a/scripts/ci/prek/check_conf_import_in_providers.py +++ /dev/null @@ -1,128 +0,0 @@ -#!/usr/bin/env python -# -# 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. -# /// script -# requires-python = ">=3.10,<3.11" -# dependencies = [ -# "rich>=13.6.0", -# ] -# /// -""" -Check that provider files import ``conf`` only from ``airflow.providers.common.compat.sdk``. - -Providers must not import ``conf`` directly from ``airflow.configuration`` or -``airflow.sdk.configuration``. The compat SDK re-exports ``conf`` and ensures -the code works across Airflow 2 and 3. -""" - -from __future__ import annotations - -import argparse -import sys -from pathlib import Path - -from common_prek_utils import console, get_imports_from_file - -# Fully-qualified import names that are forbidden (as returned by get_imports_from_file) -FORBIDDEN_CONF_IMPORTS = { - "airflow.configuration.conf", - "airflow.sdk.configuration.conf", -} - -# Executor files run inside Airflow-Core, so they may use airflow.configuration directly. -# Only airflow.sdk.configuration remains forbidden for them. -EXECUTOR_ALLOWED_CONF_IMPORTS = { - "airflow.configuration.conf", -} - -ALLOWED_IMPORT = "from airflow.providers.common.compat.sdk import conf" - - -def is_excluded(path: Path) -> bool: - """Check if a file is the compat SDK module itself (which must define the re-export).""" - return path.as_posix().endswith("airflow/providers/common/compat/sdk.py") - - -def is_executor_file(path: Path) -> bool: - """Check if a file is an executor module (lives under an ``executors/`` directory).""" - return "executors" in path.parts - - -def find_forbidden_conf_imports(path: Path) -> list[str]: - """Return forbidden conf imports found in *path*.""" - imports = get_imports_from_file(path, only_top_level=False) - forbidden = FORBIDDEN_CONF_IMPORTS - if is_executor_file(path): - forbidden = forbidden - EXECUTOR_ALLOWED_CONF_IMPORTS - return [imp for imp in imports if imp in forbidden] - - -def parse_args(): - parser = argparse.ArgumentParser( - description="Check that provider files import conf only from the compat SDK." - ) - parser.add_argument("files", nargs="*", type=Path, help="Python source files to check.") - return parser.parse_args() - - -def main() -> int: - args = parse_args() - - if not args.files: - console.print("[yellow]No files provided.[/]") - return 0 - - provider_files = [path for path in args.files if not is_excluded(path)] - - if not provider_files: - return 0 - - errors: list[str] = [] - - for path in provider_files: - try: - forbidden = find_forbidden_conf_imports(path) - except Exception as e: - console.print(f"[red]Failed to parse {path}: {e}[/]") - return 2 - - if forbidden: - errors.append(f"\n[red]{path}:[/]") - for imp in forbidden: - errors.append(f" - {imp}") - - if errors: - console.print("\n[red]Some provider files import conf from forbidden modules![/]\n") - console.print( - "[yellow]Provider files must import conf from the compat SDK:[/]\n" - f" {ALLOWED_IMPORT}\n" - "\n[yellow]The following imports are forbidden:[/]\n" - " - from airflow.configuration import conf\n" - " - from airflow.sdk.configuration import conf\n" - ) - console.print("[red]Found forbidden imports in:[/]") - for error in errors: - console.print(error) - return 1 - - console.print("[green]All provider files import conf from the correct module![/]") - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/scripts/tests/ci/prek/test_check_conf_import_in_providers.py b/scripts/tests/ci/prek/test_check_conf_import_in_providers.py deleted file mode 100644 index 647739ac44866..0000000000000 --- a/scripts/tests/ci/prek/test_check_conf_import_in_providers.py +++ /dev/null @@ -1,137 +0,0 @@ -# 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. -from __future__ import annotations - -from pathlib import Path - -import pytest -from check_conf_import_in_providers import find_forbidden_conf_imports, is_excluded, is_executor_file - - -class TestIsExcluded: - @pytest.mark.parametrize( - "path, expected", - [ - pytest.param( - "providers/common/compat/src/airflow/providers/common/compat/sdk.py", - True, - id="compat-sdk-module", - ), - pytest.param( - "providers/amazon/src/airflow/providers/amazon/hooks/s3.py", - False, - id="regular-provider-file", - ), - ], - ) - def test_is_excluded(self, path: str, expected: bool): - assert is_excluded(Path(path)) is expected - - -class TestIsExecutorFile: - @pytest.mark.parametrize( - "path, expected", - [ - pytest.param( - "providers/edge3/src/airflow/providers/edge3/executors/edge_executor.py", - True, - id="executor-file", - ), - pytest.param( - "providers/celery/src/airflow/providers/celery/executors/celery_executor.py", - True, - id="celery-executor-file", - ), - pytest.param( - "providers/amazon/src/airflow/providers/amazon/hooks/s3.py", - False, - id="regular-provider-file", - ), - ], - ) - def test_is_executor_file(self, path: str, expected: bool): - assert is_executor_file(Path(path)) is expected - - -class TestFindForbiddenConfImports: - @pytest.mark.parametrize( - "code, expected", - [ - pytest.param( - "from airflow.configuration import conf\n", - ["airflow.configuration.conf"], - id="from-airflow-configuration", - ), - pytest.param( - "from airflow.sdk.configuration import conf\n", - ["airflow.sdk.configuration.conf"], - id="from-airflow-sdk-configuration", - ), - pytest.param( - "from airflow.configuration import conf as global_conf\n", - ["airflow.configuration.conf"], - id="aliased-conf", - ), - pytest.param( - "def foo():\n from airflow.configuration import conf\n", - ["airflow.configuration.conf"], - id="inside-function", - ), - pytest.param( - "from __future__ import annotations\n" - "from typing import TYPE_CHECKING\n" - "if TYPE_CHECKING:\n" - " from airflow.sdk.configuration import conf\n", - ["airflow.sdk.configuration.conf"], - id="inside-type-checking", - ), - ], - ) - def test_forbidden_imports(self, tmp_path: Path, code: str, expected: list[str]): - f = tmp_path / "example.py" - f.write_text(code) - assert find_forbidden_conf_imports(f) == expected - - @pytest.mark.parametrize( - "code", - [ - pytest.param("from airflow.providers.common.compat.sdk import conf\n", id="compat-sdk"), - pytest.param("from airflow.configuration import has_option\n", id="other-config-attr"), - pytest.param("from airflow.configuration import AirflowConfigParser\n", id="config-parser"), - pytest.param("from airflow.providers.amazon.hooks.s3 import S3Hook\n", id="provider-import"), - pytest.param("import os\nimport sys\n", id="stdlib-only"), - pytest.param("x = 1\n", id="no-imports"), - ], - ) - def test_allowed_imports(self, tmp_path: Path, code: str): - f = tmp_path / "example.py" - f.write_text(code) - assert find_forbidden_conf_imports(f) == [] - - def test_executor_allows_airflow_configuration_conf(self, tmp_path: Path): - executor_dir = tmp_path / "executors" - executor_dir.mkdir() - f = executor_dir / "my_executor.py" - f.write_text("from airflow.configuration import conf\n") - assert find_forbidden_conf_imports(f) == [] - - def test_executor_still_forbids_sdk_configuration_conf(self, tmp_path: Path): - executor_dir = tmp_path / "executors" - executor_dir.mkdir() - f = executor_dir / "my_executor.py" - f.write_text("from airflow.sdk.configuration import conf\n") - assert find_forbidden_conf_imports(f) == ["airflow.sdk.configuration.conf"]