Skip to content

Add prek checks for cross-distribution import boundaries#65880

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:extract-65621-shared-import-checks
Apr 26, 2026
Merged

Add prek checks for cross-distribution import boundaries#65880
potiuk merged 1 commit into
apache:mainfrom
potiuk:extract-65621-shared-import-checks

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented Apr 26, 2026

Extracts the parts of #65621 that pass on current main into the existing
check-shared-distributions-structure prek hook, so we can land the
boundary-checking infrastructure now without waiting for the residual
import cleanups it surfaces.

What's added

Four AST-based checks, all gated to skip imports nested in
if TYPE_CHECKING: blocks:

  1. Shared distributions must not depend on apache-airflow*
    covers regular, optional, and PEP 735 dependency-groups.
  2. Shared distributions must not import from airflow.* in
    src/ or tests/. Error message includes the standard
    relative-import / move-to-another-distribution fix guidance.
  3. task-sdk and airflow-core must not import from the
    airflow_shared namespace
    — that namespace is the
    consumer-facing PyPI shape; in-monorepo code uses the
    airflow.sdk._shared.* / airflow._shared.* symlinks.
  4. task-sdk may only import _shared through
    airflow.sdk._shared.*
    (not airflow._shared.*). Caught one
    residual offender (task-sdk/.../deadline.py); fixed in this PR.

The equivalent allowed-prefix check for airflow-core is
intentionally deferred — airflow-core still imports
SecretsMasker and a couple of friends from airflow.sdk._shared,
and that requires a code move before the check can be turned on.
That part stays in #65621 to land once the imports are relocated.

Also bundled (small, related):

  • Expanded the hook's file filter so it fires for changes under
    airflow-core/, task-sdk/, _shared/ and any pyproject.toml
    not only files under shared/.
  • Added scripts/ci/prek to breeze's FULL_TESTS_NEEDED_FILES
    (and flipped the test that asserted otherwise) so prek-hook
    changes trigger the full test matrix.

Test plan

  • prek run check-shared-distributions-structure --all-files passes
  • pytest dev/breeze/tests/test_selective_checks.py::test_full_test_needed_when_scripts_changes passes
  • CI

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

The existing ``check-shared-distributions-structure`` prek hook only
validated the structural shape of each shared distribution
(pyproject.toml, ``src/airflow_shared/<pkg>``, ruff rules). It did
not catch imports that violate distribution boundaries.

Extends the hook with four AST-based checks that pass on current main:

1. Shared distributions must not list ``apache-airflow``,
   ``apache-airflow-core`` or ``apache-airflow-task-sdk`` as
   dependencies (regular, optional, or PEP 735 dependency-groups).

2. Shared distributions must not import from ``airflow.*`` in
   ``src/`` or ``tests/``. Includes guidance on the relative-import
   or distribution-move fix when violations are found.

3. ``task-sdk`` and ``airflow-core`` must not import from the
   ``airflow_shared`` PyPI namespace — that is the consumer-facing
   namespace; in-monorepo code should reach shared code through
   ``airflow.sdk._shared.*`` and ``airflow._shared.*`` symlinks.

4. ``task-sdk`` may only import ``_shared`` symbols through its own
   ``airflow.sdk._shared.*`` prefix, not through ``airflow._shared.*``.
   Caught one residual offender — ``deadline.py`` — fixed in this
   commit.

The equivalent allowed-prefix check for ``airflow-core`` is
intentionally deferred: ``airflow-core`` still imports
``SecretsMasker`` and friends from ``airflow.sdk._shared``, and
that needs a code move before the check can be enabled. Tracked
in apache#65621.

All checks skip imports nested inside ``if TYPE_CHECKING:`` blocks
since those are type-only and never resolved at runtime.

Also:

- Expand the hook's file filter to fire when files under
  ``airflow-core/``, ``task-sdk/``, ``_shared/`` or any
  ``pyproject.toml`` change — not just files under ``shared/``.
- Add ``scripts/ci/prek`` to the breeze selective-checks
  FULL_TESTS_NEEDED list so changes to prek hooks trigger the
  full test matrix (and the test that asserted otherwise is
  flipped to match).

Extracted from the passing parts of apache#65621.
@potiuk potiuk merged commit fd36d77 into apache:main Apr 26, 2026
141 checks passed
@potiuk potiuk deleted the extract-65621-shared-import-checks branch April 26, 2026 20:47
@potiuk potiuk removed the backport-to-v3-2-test Mark PR with this label to backport to v3-2-test branch label Apr 26, 2026
potiuk added a commit to potiuk/airflow that referenced this pull request Apr 26, 2026
Builds on the boundary checks landed in apache#65880 by enabling the parts
that still surface real violations:

- Refactor task-sdk and airflow-core distribution checks through a
  shared `check_distribution()` helper that calls both
  `check_no_airflow_shared_imports` and `check_only_allowed_shared_imports`.
  This turns on the `airflow.sdk._shared` allowed-prefix gate inside
  airflow-core, which flags the residual `SecretsMasker` and friends
  that need to be relocated.
- Add `check_devel_common_distribution()` that bans `airflow.*`
  imports from `devel-common/src/`. devel-common should reach airflow
  internals through `airflow_shared` (or move the helper into
  devel-common itself).
- Drop TODO markers next to the two `airflow.sdk._shared.secrets_masker`
  imports in airflow-core's CLI (`task_command.py`, `triggerer_command.py`)
  pointing to the followups that will let the gate go fully green.
- Add wording to the breeze provider-dependency-bump warning so it
  mentions moving the dependency above `# Additional devel dependencies`.

The hook will fail until the residual offenders in airflow-core and
devel-common are resolved — that is the intent. Reopened from apache#58825
after rebasing onto latest main.

Generated-by: Claude Code (Opus 4.7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants