From 0b2bee3841c621074760a8530f733d112e4dffc1 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Mon, 2 Dec 2024 22:43:53 +0100 Subject: [PATCH 1/2] Remove Provider Deprecations in Docker --- .../airflow/providers/docker/CHANGELOG.rst | 17 +++ .../providers/docker/decorators/docker.py | 20 +--- .../providers/docker/operators/docker.py | 34 ------ .../tests/docker/decorators/test_docker.py | 37 +------ .../tests/docker/operators/test_docker.py | 100 ------------------ 5 files changed, 19 insertions(+), 189 deletions(-) diff --git a/providers/src/airflow/providers/docker/CHANGELOG.rst b/providers/src/airflow/providers/docker/CHANGELOG.rst index 0f17ba19a2ac0..5d17b0d996bb9 100644 --- a/providers/src/airflow/providers/docker/CHANGELOG.rst +++ b/providers/src/airflow/providers/docker/CHANGELOG.rst @@ -27,6 +27,23 @@ Changelog --------- +4.0.0 +...... + +Breaking changes +~~~~~~~~~~~~~~~~ + +.. warning:: + All deprecated classes, parameters and features have been removed from the Kubernetes provider package. + The following breaking changes were introduced: + + * Decorators + * Deprecated parameter ``use_dill`` was removed. Please use ``serializer='dill'`` instead. + * Operators + * Deprecated parameter ``use_dill`` was removed. Please use ``serializer='dill'`` instead. + * Deprecated parameter ``skip_exit_code`` was removed. Please use ``skip_on_exit_code`` instead. + * Deprecated method ``get_hook()`` was removed. Please use ``hook`` property instead. + 3.14.1 ...... diff --git a/providers/src/airflow/providers/docker/decorators/docker.py b/providers/src/airflow/providers/docker/decorators/docker.py index 6863020c73adc..560028c16ed6c 100644 --- a/providers/src/airflow/providers/docker/decorators/docker.py +++ b/providers/src/airflow/providers/docker/decorators/docker.py @@ -18,13 +18,12 @@ import base64 import os -import warnings from collections.abc import Sequence from tempfile import TemporaryDirectory from typing import TYPE_CHECKING, Any, Callable, Literal from airflow.decorators.base import DecoratedOperator, task_decorator_factory -from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning +from airflow.exceptions import AirflowException from airflow.providers.common.compat.standard.utils import write_python_script from airflow.providers.docker.operators.docker import DockerOperator @@ -110,9 +109,6 @@ class _DockerDecoratedOperator(DecoratedOperator, DockerOperator): this requires to include cloudpickle in your requirements. - ``"dill"``: Use dill for serialize more complex types, this requires to include dill in your requirements. - :param use_dill: Deprecated, use ``serializer`` instead. Whether to use dill to serialize - the args and result (pickle is default). This allows more complex types - but requires you to include dill in your requirements. """ custom_operator_name = "@task.docker" @@ -121,24 +117,11 @@ class _DockerDecoratedOperator(DecoratedOperator, DockerOperator): def __init__( self, - use_dill=False, python_command="python3", expect_airflow: bool = True, serializer: Serializer | None = None, **kwargs, ) -> None: - if use_dill: - warnings.warn( - "`use_dill` is deprecated and will be removed in a future version. " - "Please provide serializer='dill' instead.", - AirflowProviderDeprecationWarning, - stacklevel=3, - ) - if serializer: - raise AirflowException( - "Both 'use_dill' and 'serializer' parameters are set. Please set only one of them" - ) - serializer = "dill" serializer = serializer or "pickle" if serializer not in _SERIALIZERS: msg = ( @@ -150,7 +133,6 @@ def __init__( command = "placeholder command" self.python_command = python_command self.expect_airflow = expect_airflow - self.use_dill = serializer == "dill" self.serializer: Serializer = serializer super().__init__( diff --git a/providers/src/airflow/providers/docker/operators/docker.py b/providers/src/airflow/providers/docker/operators/docker.py index d232cb103900e..a7e8375c91e67 100644 --- a/providers/src/airflow/providers/docker/operators/docker.py +++ b/providers/src/airflow/providers/docker/operators/docker.py @@ -23,28 +23,24 @@ import os import pickle import tarfile -import warnings from collections.abc import Container, Iterable, Sequence from functools import cached_property from io import BytesIO, StringIO from tempfile import TemporaryDirectory from typing import TYPE_CHECKING -from deprecated.classic import deprecated from docker.constants import DEFAULT_TIMEOUT_SECONDS from docker.errors import APIError from docker.types import LogConfig, Mount, Ulimit from dotenv import dotenv_values from typing_extensions import Literal -from airflow.exceptions import AirflowProviderDeprecationWarning from airflow.models import BaseOperator from airflow.providers.docker.exceptions import ( DockerContainerFailedException, DockerContainerFailedSkipException, ) from airflow.providers.docker.hooks.docker import DockerHook -from airflow.utils.types import NOTSET, ArgNotSet if TYPE_CHECKING: from logging import Logger @@ -255,33 +251,8 @@ def __init__( skip_on_exit_code: int | Container[int] | None = None, port_bindings: dict | None = None, ulimits: list[Ulimit] | None = None, - # deprecated, no need to include into docstring - skip_exit_code: int | Container[int] | ArgNotSet = NOTSET, **kwargs, ) -> None: - if skip_exit_code is not NOTSET: - warnings.warn( - "`skip_exit_code` is deprecated and will be removed in the future. " - "Please use `skip_on_exit_code` instead.", - AirflowProviderDeprecationWarning, - stacklevel=2, - ) - if skip_on_exit_code is not None and skip_exit_code != skip_on_exit_code: - msg = ( - f"Conflicting `skip_on_exit_code` provided, " - f"skip_on_exit_code={skip_on_exit_code!r}, skip_exit_code={skip_exit_code!r}." - ) - raise ValueError(msg) - skip_on_exit_code = skip_exit_code # type: ignore[assignment] - if isinstance(auto_remove, bool): - warnings.warn( - "bool value for `auto_remove` is deprecated and will be removed in the future. " - "Please use 'never', 'success', or 'force' instead", - AirflowProviderDeprecationWarning, - stacklevel=2, - ) - auto_remove = "success" if auto_remove else "never" - super().__init__(**kwargs) self.api_version = api_version if not auto_remove or auto_remove not in ("never", "success", "force"): @@ -365,11 +336,6 @@ def hook(self) -> DockerHook: timeout=self.timeout, ) - @deprecated(reason="use `hook` property instead.", category=AirflowProviderDeprecationWarning) - def get_hook(self) -> DockerHook: - """Create and return an DockerHook (cached).""" - return self.hook - @property def cli(self) -> APIClient: return self.hook.api_client diff --git a/providers/tests/docker/decorators/test_docker.py b/providers/tests/docker/decorators/test_docker.py index 5d961d0ec4e2b..158d53b71a30e 100644 --- a/providers/tests/docker/decorators/test_docker.py +++ b/providers/tests/docker/decorators/test_docker.py @@ -23,7 +23,7 @@ import pytest from airflow.decorators import setup, task, teardown -from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning +from airflow.exceptions import AirflowException from airflow.models import TaskInstance from airflow.models.dag import DAG from airflow.utils import timezone @@ -332,28 +332,6 @@ def f(): last_line_of_docker_operator_log = log_content.splitlines()[-1] assert "ValueError: This task is expected to fail" in last_line_of_docker_operator_log - @pytest.mark.parametrize( - "serializer", - [ - pytest.param("pickle", id="pickle"), - pytest.param("dill", marks=DILL_MARKER, id="dill"), - pytest.param("cloudpickle", marks=CLOUDPICKLE_MARKER, id="cloudpickle"), - ], - ) - def test_ambiguous_serializer(self, dag_maker, serializer): - @task.docker(image="python:3.9-slim", auto_remove="force", use_dill=True, serializer=serializer) - def f(): - pass - - with dag_maker(): - with pytest.warns( - AirflowProviderDeprecationWarning, match="`use_dill` is deprecated and will be removed" - ): - with pytest.raises( - AirflowException, match="Both 'use_dill' and 'serializer' parameters are set" - ): - f() - def test_invalid_serializer(self, dag_maker): @task.docker(image="python:3.9-slim", auto_remove="force", serializer="airflow") def f(): @@ -413,16 +391,3 @@ def f(): with dag_maker(): f() - - @DILL_MARKER - def test_add_dill_use_dill(self, dag_maker): - @task.docker(image="python:3.9-slim", auto_remove="force", use_dill=True) - def f(): - """Ensure dill is correctly installed.""" - import dill # noqa: F401 - - with dag_maker(): - with pytest.warns( - AirflowProviderDeprecationWarning, match="`use_dill` is deprecated and will be removed" - ): - f() diff --git a/providers/tests/docker/operators/test_docker.py b/providers/tests/docker/operators/test_docker.py index 8919fc0962d6d..8a2de2d4220fa 100644 --- a/providers/tests/docker/operators/test_docker.py +++ b/providers/tests/docker/operators/test_docker.py @@ -29,7 +29,6 @@ from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning, AirflowSkipException from airflow.providers.docker.exceptions import DockerContainerFailedException from airflow.providers.docker.operators.docker import DockerOperator, fetch_logs -from airflow.utils.task_instance_session import set_current_task_instance_session TEST_CONN_ID = "docker_test_connection" TEST_DOCKER_URL = "unix://var/run/docker.test.sock" @@ -750,49 +749,6 @@ def test_auto_remove_invalid(self, auto_remove): with pytest.raises(ValueError, match="Invalid `auto_remove` value"): DockerOperator(task_id="test", image="test", auto_remove=auto_remove) - @pytest.mark.parametrize( - "skip_exit_code, skip_on_exit_code, expected", - [ - pytest.param(101, None, [101], id="skip-on-exit-code-not-set"), - pytest.param(102, 102, [102], id="skip-on-exit-code-same"), - ], - ) - def test_skip_exit_code_fallback(self, skip_exit_code, skip_on_exit_code, expected): - warning_match = "`skip_exit_code` is deprecated and will be removed in the future." - - with pytest.warns(AirflowProviderDeprecationWarning, match=warning_match): - op = DockerOperator( - task_id="test", - image="test", - skip_exit_code=skip_exit_code, - skip_on_exit_code=skip_on_exit_code, - ) - assert op.skip_on_exit_code == expected - - @pytest.mark.parametrize( - "skip_exit_code, skip_on_exit_code", - [ - pytest.param(103, 0, id="skip-on-exit-code-zero"), - pytest.param(104, 105, id="skip-on-exit-code-not-same"), - ], - ) - def test_skip_exit_code_invalid(self, skip_exit_code, skip_on_exit_code): - warning_match = "`skip_exit_code` is deprecated and will be removed in the future." - error_match = "Conflicting `skip_on_exit_code` provided" - - with pytest.warns(AirflowProviderDeprecationWarning, match=warning_match): - with pytest.raises(ValueError, match=error_match): - DockerOperator(task_id="test", image="test", skip_exit_code=103, skip_on_exit_code=104) - - with pytest.warns(AirflowProviderDeprecationWarning, match=warning_match): - with pytest.raises(ValueError, match=error_match): - DockerOperator( - task_id="test", - image="test", - skip_exit_code=skip_exit_code, - skip_on_exit_code=skip_on_exit_code, - ) - def test_respect_docker_host_env(self, monkeypatch): monkeypatch.setenv("DOCKER_HOST", "tcp://docker-host-from-env:2375") operator = DockerOperator(task_id="test", image="test") @@ -810,62 +766,6 @@ def test_docker_host_env_unset(self, monkeypatch): operator = DockerOperator(task_id="test", image="test") assert operator.docker_url == "unix://var/run/docker.sock" - @pytest.mark.db_test - @pytest.mark.parametrize( - "skip_exit_code, skip_on_exit_code, expected", - [ - pytest.param(101, None, [101], id="skip-on-exit-code-not-set"), - pytest.param(102, 102, [102], id="skip-on-exit-code-same"), - ], - ) - def test_partial_deprecated_skip_exit_code( - self, skip_exit_code, skip_on_exit_code, expected, dag_maker, session - ): - with dag_maker(dag_id="test_partial_deprecated_skip_exit_code", session=session): - DockerOperator.partial( - task_id="fake-task-id", - skip_exit_code=skip_exit_code, - skip_on_exit_code=skip_on_exit_code, - ).expand(image=["test", "apache/airflow"]) - - dr = dag_maker.create_dagrun() - tis = dr.get_task_instances(session=session) - with set_current_task_instance_session(session=session): - warning_match = r"`skip_exit_code` is deprecated and will be removed" - for ti in tis: - with pytest.warns(AirflowProviderDeprecationWarning, match=warning_match): - ti.render_templates() - assert ti.task.skip_on_exit_code == expected - - @pytest.mark.db_test - @pytest.mark.parametrize( - "skip_exit_code, skip_on_exit_code", - [ - pytest.param(103, 0, id="skip-on-exit-code-zero"), - pytest.param(104, 105, id="skip-on-exit-code-not-same"), - ], - ) - def test_partial_deprecated_skip_exit_code_ambiguous( - self, skip_exit_code, skip_on_exit_code, dag_maker, session - ): - with dag_maker("test_partial_deprecated_skip_exit_code_ambiguous", session=session): - DockerOperator.partial( - task_id="fake-task-id", - skip_exit_code=skip_exit_code, - skip_on_exit_code=skip_on_exit_code, - ).expand(image=["test", "apache/airflow"]) - - dr = dag_maker.create_dagrun(session=session) - tis = dr.get_task_instances(session=session) - with set_current_task_instance_session(session=session): - warning_match = r"`skip_exit_code` is deprecated and will be removed" - for ti in tis: - with ( - pytest.warns(AirflowProviderDeprecationWarning, match=warning_match), - pytest.raises(ValueError, match="Conflicting `skip_on_exit_code` provided"), - ): - ti.render_templates() - @pytest.mark.parametrize( "log_lines, expected_lines", [ From 8bcf1cef91e5fd6166ac86103bc1d160fdeca732 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Mon, 2 Dec 2024 23:15:37 +0100 Subject: [PATCH 2/2] Fix mypy and pytest --- providers/tests/docker/decorators/test_docker.py | 1 - providers/tests/docker/operators/test_docker.py | 16 +--------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/providers/tests/docker/decorators/test_docker.py b/providers/tests/docker/decorators/test_docker.py index 158d53b71a30e..5372da374b93b 100644 --- a/providers/tests/docker/decorators/test_docker.py +++ b/providers/tests/docker/decorators/test_docker.py @@ -261,7 +261,6 @@ def g(): assert some_task.command == clone_of_docker_operator.command assert some_task.expect_airflow == clone_of_docker_operator.expect_airflow assert some_task.serializer == clone_of_docker_operator.serializer - assert some_task.use_dill == clone_of_docker_operator.use_dill assert some_task.pickling_library is clone_of_docker_operator.pickling_library def test_respect_docker_host_env(self, monkeypatch, dag_maker): diff --git a/providers/tests/docker/operators/test_docker.py b/providers/tests/docker/operators/test_docker.py index 8a2de2d4220fa..2e9b001959279 100644 --- a/providers/tests/docker/operators/test_docker.py +++ b/providers/tests/docker/operators/test_docker.py @@ -26,7 +26,7 @@ from docker.errors import APIError from docker.types import DeviceRequest, LogConfig, Mount, Ulimit -from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning, AirflowSkipException +from airflow.exceptions import AirflowException, AirflowSkipException from airflow.providers.docker.exceptions import DockerContainerFailedException from airflow.providers.docker.operators.docker import DockerOperator, fetch_logs @@ -82,8 +82,6 @@ def test_hook_usage(docker_hook_patcher, docker_conn_id, tls_params: dict): **tls_params, ) hook = op.hook - with pytest.warns(AirflowProviderDeprecationWarning, match="use `hook` property instead"): - assert hook is op.get_hook() docker_hook_patcher.assert_called_once_with( docker_conn_id=docker_conn_id, @@ -729,18 +727,6 @@ def test_ulimits(self): assert "ulimits" in self.client_mock.create_host_config.call_args.kwargs assert ulimits == self.client_mock.create_host_config.call_args.kwargs["ulimits"] - @pytest.mark.parametrize( - "auto_remove, expected", - [ - pytest.param(True, "success", id="true"), - pytest.param(False, "never", id="false"), - ], - ) - def test_bool_auto_remove_fallback(self, auto_remove, expected): - with pytest.warns(AirflowProviderDeprecationWarning, match="bool value for `auto_remove`"): - op = DockerOperator(task_id="test", image="test", auto_remove=auto_remove) - assert op.auto_remove == expected - @pytest.mark.parametrize( "auto_remove", ["True", "false", pytest.param(None, id="none"), pytest.param(None, id="empty"), "here-and-now"],