Add sphinx-airflow-theme as uv workspace member#65840
Conversation
02b9bc6 to
4bacb0f
Compare
|
How are you going to make sure it is going to be synchronized between |
potiuk
left a comment
There was a problem hiding this comment.
Waiting for explantion of how the process shoudl work
4bacb0f to
7dc298f
Compare
dd88bf6 to
26ed34d
Compare
571eef7 to
d8c747c
Compare
d8c747c to
41280fd
Compare
41280fd to
cfddb02
Compare
|
Hmm - I think I have one more proposal to make "documentation editing" works equally well for "versioned docs" in airflow and "airflow-site" for all the webpage stuff. We basically want to have two workflows:
I think that should be possilbe by: a) locally building (not fetching) the _gen files - when we always fetch them from whl file, we cannot easily iterate locally on thme b) synchronizing - rather than pulling the "airflow-sphinx-theme" between "airflow" and "airflow-site" - that means two -way operations:
Do you thin kit's a good and workable approach @srchilukoori ? If so - can you also implement it and add documentaiton to the docs building explaining this process (i.e. what you need to do in order to modify themes when you work on ailrflow, and what happens when you do it for "airflow-site". |
cfddb02 to
778d7e3
Compare
I added the theme files to the fetch_theme_assets.py, the version bump will pull all files (new + existing). Adding the local build and bi-directional sync will add more complexity, so I prefer to keep it to forward sync from airflow-site to airflow. |
778d7e3 to
1fa3a98
Compare
potiuk
left a comment
There was a problem hiding this comment.
Thanks for the update — the version concern from my last pass is resolved: it's now __version__ = "0.3.11" and the dependency has moved from the URL-pinned wheel to the workspace member (sphinx-airflow-theme>=0.3.11), so fetch_theme_assets.py no longer resolves a stale/older asset. Good.
One thing before this can move: the branch is currently conflicting with main (DIRTY), so please rebase onto latest main. The two failing checks (Latest Boto and Pendulum2 provider tests) are unrelated to a docs-theme change and look like stale-base fallout — a rebase should clear both. Once it's rebased and green, ping me for a full re-review of the workspace-member wiring.
Not approving in the meantime — purely the rebase, not the approach.
This review was drafted by an AI-assisted tool and confirmed by an Airflow maintainer. The findings below are observations, not blockers; an 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 Airflow handles maintainer review: contributing-docs/05_pull_requests.rst.
1fa3a98 to
2da996f
Compare
rebased the branch and resolved the conflicts. ci tests are running now. |
d2136a9 to
fdc5cf1
Compare
fdc5cf1 to
24973e2
Compare
|
Hmm:
When I look at that - I think we should do locoal building of assets in airflow repo (it's really needed when you want to locally modiy the theme and see it "live" - that was the main motivation of the change. I do not think they should be pulled from wheel - it does not solve any issue to be honest. |
24973e2 to
a458aec
Compare
@potiuk Having the site files in this repo, would lead to code divergence between the site and the airflow repository. Are you planning to merge the airflow-site into this repository? The current state (in this PR) is keeping the airflow-site as SOT, and pulling new versions when there are updates to airflow-site. |
b87a9b9 to
e55304c
Compare
Vendor sphinx-airflow-theme into docs-theme/ as a uv workspace member. Templates are committed; compiled assets and template updates are synced from the published wheel by fetch_theme_assets.py, which extracts all theme files except __init__.py (version source of truth, managed by upgrade_important_versions.py). airflow-site remains the single source of truth — no reverse sync, no local asset builds. Fetch theme assets on host before doc builds in CI The MOUNT_ALL bind mount shadows _gen/ baked into the Docker image, so the host must have the assets before breeze build-docs runs.
e55304c to
7df547b
Compare
|
I really think that this is far too much to solve the issue. It does not solve the actual issue described - being able to quickly iterate n the theme changes while working on docs. Looking at the current implementation, I think a better approach would be to handle the case differently:
That would be far less intrusive - and would be an "easy" dev-env for docs - and it would solve the case where we want to be able to work on theme and see the result of it in versioned docs - while also (in airflow-site) build the docs for landing page and the website with the same theme. Then the flow would be:
I think that seem like closes to what the issue describes. |
Summary
Vendors
sphinx-airflow-themeas a uv workspace member (docs-theme/). Templates are committed; compiled assets and template updates are synced from the published wheel at build time viascripts/ci/fetch_theme_assets.py.__init__.pyis owned locally for version management viaupgrade_important_versions.py.Closes: #45576
Changes
docs-theme/— vendored theme (templates, config, static JS);_gen/gitignoredscripts/ci/fetch_theme_assets.py— downloads wheel, extracts all theme files except__init__.py(version source of truth stays local)pyproject.toml/devel-common/pyproject.toml— workspace member +>=0.3.11depDockerfile.ci/.github/workflows/airflow-distributions-tests.yml— fetch assets in CIscripts/ci/docker-compose/local.yml/.dockerignore/docker_command_utils.py— bind-mount wiringupgrade_important_versions.py— tracks theme version in__init__.pydev/breeze/tests/test_theme_workspace.py— retry test for fetch scriptWhy two fetch points?
Breeze bind-mounts
docs-theme/over the image path, masking baked content. Workflow fetches on host; Dockerfile covers non-mount scenarios.Theme sync model
airflow-site is the source of truth. When a new theme version is published,
upgrade_important_versions.pybumps__version__in__init__.py, which triggersfetch_theme_assets.pyto re-extract all theme files (templates +_gen/assets) from the new wheel. No reverse sync, no local asset builds.Was generative AI tooling used to co-author this PR?