Centralise structlog initialisation in BaseDagBundle

[dabla]

BaseDagBundle now owns the structlog logger via a lazy _log property, so subclasses no longer need to construct and bind it themselves in their constructors. I saw all Dag bundles extending this class re-implemented the same logic regarding the structlog initialization when reviewing PR #67016, hence why I created this PR to make the code more DRY.

What changed

• BaseDagBundle gains a _log lazy property that creates a structlog
  logger on first access, bound with bundle_name, version, and
  whatever _log_context() returns.
• A _log_context(self) -> dict template-method hook (returns {} by
  default) lets each subclass declare its own extra log fields without
  duplicating the boilerplate structlog.get_logger(...).bind(...) call.
• GCSDagBundle, S3DagBundle, and GitDagBundle each implement
  _log_context() and drop their manual self._log = log.bind(...) init
  blocks and the now-unused module-level / local structlog.get_logger
  calls.
• LocalDagBundle needs no change — the default _log_context() returns
  {}, which is the correct behaviour for a bundle with no extra context.

Why lazy?

BaseDagBundle.__init__ is called via super().__init__() before subclass-specific attributes (e.g. bucket_name, bare_repo_path) are set. Evaluating _log_context() at base-__init__ time would therefore fail or produce incomplete context. The lazy property defers evaluation until first use, by which point all __init__ work across the hierarchy is complete.

---

Was generative AI tooling used to co-author this PR?

• [ x ] Yes (please specify the tool below)

Claude Sonnet 4.6

---

• Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
• For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
• When adding dependency, check compliance with the ASF 3rd Party License Policy.
• For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

[shahar1]

AI Summary

Nice cleanup — the duplication across the three bundles was real and the lazy property is the right shape for it. The # use next version markers on the three pyproject.tomls are exactly per contributing-docs/13_airflow_dependencies_and_extras.rst, so those are correct as-is.

One blocking issue though: turning _log into a read-only property breaks every already-released bundle provider when users upgrade core without upgrading providers.

Blocking — _log read-only property breaks released bundle providers (airflow-core/src/airflow/dag_processing/bundles/base.py:359)

BaseDagBundle._log is today a plain instance attribute, and every bundle subclass assigns it in __init__. This PR removes those assignments in sources, but the already-released apache-airflow-providers-git, -amazon, and -google still contain:

self._log = log.bind(
    bundle_name=self.name,
    version=self.version,
    ...
)

Providers are released independently of core and pin only apache-airflow>=3.0.0 — no upper bound. So a user on airflow-core main + the currently-released git provider gets:

AttributeError: property '_log' of 'GitDagBundle' object has no setter

…at GitDagBundle.__init__, i.e. the Git/S3/GCS Dag bundle stops working entirely on core upgrade. The common.compat shim added in this PR covers the new-provider-on-old-core direction; this is the opposite direction and nothing covers it. BaseDagBundle is also a documented extension point, so the same break hits any third-party bundle that sets self._log.

Adding a setter keeps the lazy default and preserves every existing subclass — suggestion inline.

Missing test for the new behaviour in airflow-core

Flag any changed or added behaviour without a corresponding test, and flag tests that a reviewer cannot fail by reverting the PR's change. The target is exactly 100% coverage of what the PR changes — no more, no less.

— .github/instructions/code-review.instructions.md

The new _log property and the _log_context() hook land in airflow-core, but the only tests are in providers/common/compat/tests/. Nothing in airflow-core/tests/unit/dag_processing/bundles/test_base.py covers them, and — more importantly — no test anywhere asserts that a subclass's _log_context() is merged into the logger bindings, which is the whole point of the PR. test_log_is_bound_with_context only checks bundle_name and version, both of which the base already supplied. A subclass overriding _log_context() and asserting its keys land in the bindings would fail without this change; today nothing would.

A setter (per the blocking finding) deserves a test too — instantiating a bundle that assigns self._log should keep working.

Test isolation — reload() leaks into the rest of the session

test_compat_subclass_provides_log_when_missing reloads airflow.providers.common.compat.bundles inside the mock.patch.object block. When the patch exits, CoreBaseDagBundle._log is restored — but sys.modules still holds the reloaded module, whose BaseDagBundle is now permanently the compat subclass. Any later test importing it in the same session gets the fallback class instead of the modern one. Details inline.

Smaller observations

• airflow-core/src/airflow/dag_processing/bundles/base.py:355 — _log_context is a noun-only name. AGENTS.md: "Name functions and methods with action verbs: get_, extract_, find_, compute_, build_, etc. Avoid noun-only names like _serialize_keys or _base_names — they read as attributes, not callables." _get_log_context / _build_log_context reads better, and it's cheap to rename now while the hook has no external implementers.
• base.py:355 — -> dict would be better as dict[str, Any]; the _log property has no return annotation, and self.__log = None leaves the attribute typed as None (mypy only lets the later assignment through because structlog.get_logger() returns Any).
• providers/common/compat/tests/unit/common/compat/bundles/test_dag_bundle.py — _DummyBundle is defined three times, identically apart from the base class. A module-level factory (or a fixture parametrised on the base) would remove ~60 of the 129 lines.
• test_dag_bundle.py:88 — asserting on log._context reaches into structlog's private API. structlog.testing.capture_logs() gives the same signal against a public surface.
• Commit messages use Conventional Commits prefixes (refactor: …). AGENTS.md: "do not use Conventional Commits prefixes (fix:, feat:, chore:, docs:, refactor:, …). apache/airflow does not follow that convention." Cosmetic given the squash-merge takes the PR title, which is fine as-is.

---

This review was drafted by an AI-assisted tool and
confirmed by an Airflow maintainer. After you've
addressed the points above and pushed an update, an Airflow
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.

---

Drafted-by: Claude Code (Opus 4.8); reviewed by @shahar1 before posting