Skip to content

Commit

Permalink
Remove Provider Deprecations in Docker (#44583)
Browse files Browse the repository at this point in the history
* Remove Provider Deprecations in Docker

* Fix mypy and pytest
  • Loading branch information
jscheffl authored Dec 2, 2024
1 parent 9765a42 commit f1f6499
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 205 deletions.
17 changes: 17 additions & 0 deletions providers/src/airflow/providers/docker/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
......

Expand Down
20 changes: 1 addition & 19 deletions providers/src/airflow/providers/docker/decorators/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"
Expand All @@ -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 = (
Expand All @@ -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__(
Expand Down
34 changes: 0 additions & 34 deletions providers/src/airflow/providers/docker/operators/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"):
Expand Down Expand Up @@ -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
Expand Down
38 changes: 1 addition & 37 deletions providers/tests/docker/decorators/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -332,28 +331,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():
Expand Down Expand Up @@ -413,16 +390,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()
116 changes: 1 addition & 115 deletions providers/tests/docker/operators/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@
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
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"
Expand Down Expand Up @@ -83,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,
Expand Down Expand Up @@ -730,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"],
Expand All @@ -750,49 +735,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")
Expand All @@ -810,62 +752,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",
[
Expand Down

0 comments on commit f1f6499

Please sign in to comment.