Add --build-constraint(s) flag support to constraints pipeline#65511
Add --build-constraint(s) flag support to constraints pipeline#65511nailo2c wants to merge 7 commits into
--build-constraint(s) flag support to constraints pipeline#65511Conversation
60c5099 to
ff23dab
Compare
ff23dab to
0cea6f0
Compare
0cea6f0 to
9b28df3
Compare
9b28df3 to
80789d8
Compare
potiuk
left a comment
There was a problem hiding this comment.
Read through the build-constraints generator, the install-flow plumbing, and the Dockerfile/Dockerfile.ci changes. The feature is well-implemented and the test coverage on the generator (859 lines) is genuinely thorough — caching, PEP 503 normalisation, conflict-retry, sdist tar/zip handling, legacy setuptools+wheel fallback, all covered. The uv sync-exclusion path (Dockerfile.ci:1112-1114) is correctly scoped, and the explicit-vs-inferred fail-fast contract (install_airflow_and_providers.py:235-264) matches what the PR description promises.
A few observations before I'd want to sign off — none blocking, but the regex fragility in particular is worth a hardening pass.
Conflict-detection regex is brittle to future uv error-message changes (scripts/in_container/run_generate_constraints.py:809-817)
The retry loop derives the conflicting package name by regex-matching uv pip compile's stderr. If uv reformats its error wording (e.g. drops the <>=!~ operator from the rendered conflict line, switches to a structured emoji prefix, or wraps long names), this regex silently no-matches → the loop continues without retrying → the next iteration raises "uv pip compile failed" with no useful context. The tests (lines 644–772) cover the current phrasings well (markers, underscores, multiple wordings) but stay inside the assumption that the regex shape is stable.
Two cheap mitigations:
- If no conflict pkg is extracted from stderr after a failed compile, log the raw stderr at WARNING and
raiserather than swallowing — so auvupgrade that breaks the regex fails loudly with the actual stderr in the build log. - Pin a minimum
uvversion in the script's docstring / hook config and bump it whenever you re-verify the regex against a new release.
Backward-compat claim in the PR body is slightly narrower than implemented
The PR body says "Auto-inferred missing build constraints are skipped for backward compatibility with old constraint branches." That's true for inferred URLs (install_airflow_and_providers.py:259-264 warns + returns None), but explicit AIRFLOW_BUILD_CONSTRAINTS_LOCATION values fail hard with sys.exit(1) (line 235-239). That's the right behaviour — but please make it explicit in the PR body / the user-facing doc that only the inferred path is forgiving; users opting in explicitly will see hard failures on a missing file/URL, which is fine but worth flagging.
Smaller observations
scripts/in_container/install_airflow_and_providers.py:229—target = Path("/tmp/build-constraints.txt")is hardcoded whileDockerfile/Dockerfile.cidownload to${HOME}/build-constraints.txt. The two paths don't actually collide (the Python script downloads its own copy independently), but the inconsistency is a hygiene smell. Consider either using${HOME}/build-constraints.txt(matching the Dockerfile) ortempfile.NamedTemporaryFile().scripts/in_container/run_generate_constraints.py:651-652—except Exception: cache = {}discards the original parse error without logging. If the cache file is corrupted, the next run silently rescans everything; an observer has no way to know why the rescan happened. A singlelog.warning("Build-constraints cache at %s is corrupted (%s); rebuilding", cache_path, exc)before the reset would help post-mortem.airflow-core/docs/installation/installing-from-pypi.rst— the new docs explain the feature well, but don't call out that the--build-constraint(s)flag requires the correspondingpip(≥ 23.x) /uvversion. Worth a one-liner so users on olderpipdon't hit confusing errors.- Integration coverage — the unit tests are excellent for the generator's internal logic, but there's no end-to-end test that the generated
build-constraints-{python}.txtis actually consumed correctly bypip install --build-constraintagainst a real package set. Could be a CI-only smoke test against a single-package fixture.
Positive
- Generator handles tar.gz (streaming) and zip (full download) sdists defensively, with a 10k-member cap on tar to bound memory (
run_generate_constraints.py:581-630). - Cache invalidation correctly drops stale entries when
uv.lockchanges — verified by the dedicated test attest_build_constraints.py:375-403. - Dockerfile changes are pure ARG/ENV plumbing; no new install steps that affect images for users who don't opt in.
- Breeze wiring is consistent across all four entry points (ci-image, prod-image, developer/shell, release-management) — no missing surface.
This review was drafted by an AI-assisted tool and confirmed by an Apache Airflow maintainer. The findings below are observations, not blockers; an Apache Airflow maintainer — a real person — will take the next look at the PR. If you think a finding is mis-applied, please reply on the PR and a maintainer will weigh in.
More on how Apache Airflow handles maintainer review: contributing-docs/05_pull_requests.rst.
Makes sense-- I've checked that Here is an idea, we can add a smoke test like below: @pytest.mark.integration
def test_conflict_auto_resolution_with_real_uv(tmp_path):
"""Smoke test: real uv with a known conflict - proves the regex
actually matches uv's current stderr format."""
build_reqs = {
"pkg_a": {"cython>=3.0,<3.1"},
"pkg_b": {"cython>=3.1.2"},
}
output_path = tmp_path / "build-constraints.txt"
_resolve_build_requirements(build_reqs, output_path, config_params=...)
assert output_path.exists() Then we can detect failure through the canary run.
Already fixed in docs and PR description.
I chose
Already added a warning log.
Fixed.
Already added an E2E test. |
|
@nailo2c — Removing the The label's contract is that the PR is ready for maintainer review — a regression like this means the PR temporarily isn't. Rebase your branch onto the latest
No rush. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
f8e6880 to
91dd704
Compare
91dd704 to
de520b2
Compare
closes: #54394
Hi folks, I know this is a big PR. I'll try my best to explain it concise and clear.
Summary
Add build constraints support to Airflow's constraints pipeline.
Airflow now generates and publishes
build-constraints-{python}.txtnext to the existing runtime constraints, and install flows pass them touv pip install --build-constraintsorpip install --build-constraintwhen available.How it works
flowchart LR A["Existing inputs:<br/>uv.lock + workspace pyproject.toml"] B["Existing runtime constraints"] C["Existing constraints branch"] D["Existing install commands (uv pip install / pip install)"] X["NEW build constraints generation"] Y["NEW build-constraints-3.x.txt"] Z["NEW --build-constraints flags"] A --> B --> C --> D A --> X --> Y --> C C --> Z --> D classDef existing fill:#ffffff,stroke:#777777,color:#111111; classDef added fill:#e7f5ff,stroke:#1971c2,stroke-width:2px,color:#111111; class A,B,C,D existing; class X,Y,Z added;Key design decisions:
Build constraints are a single file per Python version, shared across all 3 constraint modes.
uv syncpaths are excluded becauseuv syncdoes not support--build-constraints.Fallback no-constraints paths never include build constraints, matching runtime constraints behavior.
Missing auto-inferred build constraints are skipped with a warning for backward compatibility with old constraint branches, while explicitly configured build constraints fail fast if the file or URL is not accessible.
Explicit
AIRFLOW_BUILD_CONSTRAINTS_LOCATIONvalues fail fast if the file or URL is missing.The generator targets build requirements relevant to Airflow's default wheel-preferred install path, rather than forcing source-build coverage for every locked package.
Source-scan scope validation
A separate full-scan script scans all ~749 packages with sdist URLs (no wheel filtering, no cache). Running both against the same
uv.lock:All 37 extra deps come from packages with universal wheels. Under the default wheel-preferred install path, those packages are not expected to enter build isolation, so the production scanner covers the build deps relevant to normal Airflow installs while keeping generation fast.
Review guide
Suggested review order follows the data flow:
scripts/in_container/install_airflow_and_providers.py
scripts/in_container/install_development_dependencies.py
scripts/ci/constraints/*.sh
MANUALLY_GENERATING_IMAGE_CACHE_AND_CONSTRAINTS.md
09_release_management_tasks.rst
Generated/synced files:
prek update-inlined-dockerfile-scripts --all-files.Verification
pyproject.tomlfiles, and uv conflict diagnostic wording.build-constraints-*.txtfor all three constraints modes.Generation: workspace scan, upstream scan, and constraint resolution
Integration: 3 constraint modes all produce build-constraints-*.txt
Negative & positive tests: build constraints actually block/allow installation
CI Results
We can find the build constraints related log in this PR's CI run.
Was generative AI tooling used to co-author this PR?
Claude Opus 4.6 + Codex 5.4
{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.