From 49e1f4d5b5308df51e21f270d22707688fb57eab Mon Sep 17 00:00:00 2001 From: Andrey Anshin Date: Tue, 20 Feb 2024 15:33:10 +0400 Subject: [PATCH 1/3] Allow to skip `test_should_be_importable` if optional dependency not installed --- airflow/providers/amazon/aws/fs/s3.py | 3 +- tests/always/test_example_dags.py | 53 +++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/airflow/providers/amazon/aws/fs/s3.py b/airflow/providers/amazon/aws/fs/s3.py index 128e225815131..ee7499f575522 100644 --- a/airflow/providers/amazon/aws/fs/s3.py +++ b/airflow/providers/amazon/aws/fs/s3.py @@ -25,6 +25,7 @@ from botocore import UNSIGNED from requests import HTTPError +from airflow.exceptions import AirflowOptionalProviderFeatureException from airflow.providers.amazon.aws.hooks.s3 import S3Hook if TYPE_CHECKING: @@ -49,7 +50,7 @@ def get_fs(conn_id: str | None, storage_options: dict[str, str] | None = None) - try: from s3fs import S3FileSystem except ImportError: - raise ImportError( + raise AirflowOptionalProviderFeatureException( "Airflow FS S3 protocol requires the s3fs library, but it is not installed as it requires" "aiobotocore. Please install the s3 protocol support library by running: " "pip install apache-airflow-providers-amazon[s3fs]" diff --git a/tests/always/test_example_dags.py b/tests/always/test_example_dags.py index ffb5c4dd3b185..927e5864455b0 100644 --- a/tests/always/test_example_dags.py +++ b/tests/always/test_example_dags.py @@ -17,10 +17,18 @@ from __future__ import annotations import os +import sys from glob import glob from pathlib import Path import pytest +from packaging.specifiers import SpecifierSet +from packaging.version import Version + +if sys.version_info >= (3, 9): + from importlib.metadata import version +else: + from importlib_metadata import version from airflow.models import DagBag from airflow.utils import yaml @@ -36,6 +44,23 @@ "The test is skipped because we are running in limited Pydantic environment", allow_module_level=True ) +OPTIONAL_PROVIDERS_DEPENDENCIES = { + # This test required to be installed `s3fs`, which are not installed into some CI checks + "tests/system/providers/common/io/example_file_transfer_local_to_s3.py": {"s3fs": None} +} + + +def match_optional_dependencies(distribution_name: str, specifier: str | None) -> tuple[bool, str]: + try: + package_version = Version(version(distribution_name)) + except ImportError: + return False, f"{distribution_name!r} not installed." + + if specifier and package_version not in SpecifierSet(specifier): + return False, f"{distribution_name!r} required {specifier}, but installed {package_version}." + + return True, "" + def get_suspended_providers_folders() -> list[str]: """ @@ -54,7 +79,7 @@ def get_suspended_providers_folders() -> list[str]: return suspended_providers -def example_not_suspended_dags(): +def example_not_suspended_dags(exclude_db_exception: bool = False): example_dirs = ["airflow/**/example_dags/example_*.py", "tests/system/providers/**/example_*.py"] suspended_providers_folders = get_suspended_providers_folders() possible_prefixes = ["airflow/providers/", "tests/system/providers/"] @@ -66,8 +91,22 @@ def example_not_suspended_dags(): for example_dir in example_dirs: candidates = glob(f"{AIRFLOW_SOURCES_ROOT.as_posix()}/{example_dir}", recursive=True) for candidate in candidates: - if not candidate.startswith(tuple(suspended_providers_folders)): - yield candidate + param_marks = [] + + if candidate.startswith(tuple(suspended_providers_folders)): + param_marks.append(pytest.mark.skip(reason="Suspended provider")) + + for optional, dependencies in OPTIONAL_PROVIDERS_DEPENDENCIES.items(): + if candidate.endswith(optional): + for distribution_name, specifier in dependencies.items(): + result, reason = match_optional_dependencies(distribution_name, specifier) + if not result: + param_marks.append(pytest.mark.skip(reason=reason)) + + if exclude_db_exception and candidate.endswith(tuple(NO_DB_QUERY_EXCEPTION)): + param_marks.append(pytest.mark.skip(reason="Expected DB call")) + + yield pytest.param(candidate, marks=tuple(param_marks), id=relative_path(candidate)) def example_dags_except_db_exception(): @@ -83,8 +122,8 @@ def relative_path(path): @pytest.mark.db_test -@pytest.mark.parametrize("example", example_not_suspended_dags(), ids=relative_path) -def test_should_be_importable(example): +@pytest.mark.parametrize("example", example_not_suspended_dags()) +def test_should_be_importable(example: str): dagbag = DagBag( dag_folder=example, include_examples=False, @@ -94,8 +133,8 @@ def test_should_be_importable(example): @pytest.mark.db_test -@pytest.mark.parametrize("example", example_dags_except_db_exception(), ids=relative_path) -def test_should_not_do_database_queries(example): +@pytest.mark.parametrize("example", example_not_suspended_dags(exclude_db_exception=True)) +def test_should_not_do_database_queries(example: str): with assert_queries_count(0): DagBag( dag_folder=example, From bdeb4ece5c57aaec4a86006c5faf63781bd0422f Mon Sep 17 00:00:00 2001 From: Andrey Anshin Date: Tue, 20 Feb 2024 15:45:36 +0400 Subject: [PATCH 2/3] Rollback exception type changing --- airflow/providers/amazon/aws/fs/s3.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/airflow/providers/amazon/aws/fs/s3.py b/airflow/providers/amazon/aws/fs/s3.py index ee7499f575522..128e225815131 100644 --- a/airflow/providers/amazon/aws/fs/s3.py +++ b/airflow/providers/amazon/aws/fs/s3.py @@ -25,7 +25,6 @@ from botocore import UNSIGNED from requests import HTTPError -from airflow.exceptions import AirflowOptionalProviderFeatureException from airflow.providers.amazon.aws.hooks.s3 import S3Hook if TYPE_CHECKING: @@ -50,7 +49,7 @@ def get_fs(conn_id: str | None, storage_options: dict[str, str] | None = None) - try: from s3fs import S3FileSystem except ImportError: - raise AirflowOptionalProviderFeatureException( + raise ImportError( "Airflow FS S3 protocol requires the s3fs library, but it is not installed as it requires" "aiobotocore. Please install the s3 protocol support library by running: " "pip install apache-airflow-providers-amazon[s3fs]" From 15e06e778a09087e0b4c5aec31795a92ddeedd00 Mon Sep 17 00:00:00 2001 From: Andrey Anshin Date: Tue, 20 Feb 2024 15:50:06 +0400 Subject: [PATCH 3/3] Add desciption for OPTIONAL_PROVIDERS_DEPENDENCIES --- tests/always/test_example_dags.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/always/test_example_dags.py b/tests/always/test_example_dags.py index 927e5864455b0..0d58596366d3d 100644 --- a/tests/always/test_example_dags.py +++ b/tests/always/test_example_dags.py @@ -44,8 +44,14 @@ "The test is skipped because we are running in limited Pydantic environment", allow_module_level=True ) -OPTIONAL_PROVIDERS_DEPENDENCIES = { - # This test required to be installed `s3fs`, which are not installed into some CI checks +# Some certain of examples/system tests might require additional dependencies, +# which are not installed into specific CI check +# Format of dictionary: +# key: prefix of the file which need to be excluded, +# values: dictionary with package distributions and optional specifier, e.g. >=2.3.4 +OPTIONAL_PROVIDERS_DEPENDENCIES: dict[str, dict[str, str | None]] = { + # Regression of https://github.com/apache/airflow/pull/37524 + # It loads the module now eagerly instead of lazily "tests/system/providers/common/io/example_file_transfer_local_to_s3.py": {"s3fs": None} }