Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion airflow-core/src/airflow/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,12 @@ def __getattr__(name: str):
except KeyError:
raise AttributeError(f"module '{__name__}' has no attribute '{name}'") from None

import importlib

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: importlib is part of the import machinery and always loaded, so this can sit at the top of the file with the rest of the imports. Moot if you take the add_deprecated_classes route above.

import warnings

warnings.warn(
f"{__name__}.{name} is deprecated. Use {modpath}.{name} instead.",
DeprecationWarning,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stacklevel=2 attributes the warning to the caller's module, so a DAG author doing from airflow.utils.helpers import render_template_as_native gets a DeprecationWarning reported against their DAG file, which Python's default ignore filter drops. The action="default" filter Airflow installs in configuration.py:58 is scoped to module="airflow" and doesn't match either. I confirmed locally that after your fix the shim resolves correctly but the notice never reaches stderr.

We already have airflow.utils.deprecation_tools.add_deprecated_classes for this, and it gets both parts right: importlib.import_module plus DeprecatedImportWarning, which subclasses FutureWarning and so is shown by default. airflow/io/__init__.py uses the same __name__: {...} form. Worth replacing the hand-rolled block with:

from airflow.utils.deprecation_tools import add_deprecated_classes

add_deprecated_classes(
    {
        __name__: {
            "render_template_as_native": "airflow.sdk.definitions.context.render_template_as_native",
            "render_template_to_string": "airflow.sdk.definitions.context.render_template_to_string",
            "prevent_duplicates": "airflow.sdk.definitions.mappedoperator.prevent_duplicates",
        }
    },
    package=__name__,
)

which lets both __getattr__ and __deprecated_imports go away.

stacklevel=2,
)
return getattr(__import__(modpath), name)
return getattr(importlib.import_module(modpath), name)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a regression test? airflow-core/tests/unit/utils/test_helpers.py already imports helpers, so it's a few lines:

@pytest.mark.parametrize(
    "name", ["render_template_as_native", "render_template_to_string", "prevent_duplicates"]
)
def test_deprecated_imports_resolve(name):
    with pytest.warns(DeprecationWarning):
        assert getattr(helpers, name) is not None

Nothing in the tree touches __deprecated_imports today, which is why all three entries were broken rather than just one.