kaxil commented on PR #71477:
URL: https://github.com/apache/airflow/pull/71477#issuecomment-5735919501
Round-1 items all landed, thanks. Four follow-ups and two nits, none
blocking.
**`extract_versions.py`: the `is_guide_page` filter runs before `git_show`
now, but it removes very little at a release tag.** At
`providers-amazon/9.36.0` the docs tree holds 111 `.rst`, of which 2 are
`_`-prefixed and 2 are `changelog.rst`/`commits.rst`, so 107 `git show` spawns
remain and the per-provider-version cost from round 1 is still paid. Feeding
`<tag>:<path>` lines to a single `git cat-file --batch` on stdin builds a
byte-identical dict in 0.019s against 1.254s for the loop, with `git_ls_tree`
and `is_guide_page` unchanged. `--all-versions` pays the current cost once per
provider-version.
**`extract_parameters.py:481`: my round-1 off-by-one prompted the wrong
correction.** `attach_guide_urls` does `if not anchor: continue`, so
`guide_url` is set only for classes a guide documents, which is what the new
`test_module_contract_omits_guide_url_for_undocumented_classes` asserts.
Entries for undocumented classes carry 12 keys, not 13. "all 12 Module fields,
plus `guide_url` when a how-to guide documents the class" would be accurate.
**`test_module_contract_omits_guide_url_for_undocumented_classes` cannot
fail.** `_validate` in `registry_contract_models.py` validates and then returns
the input payload rather than `model_dump()`, so the assertion checks that a
dict built without `guide_url` has no `guide_url`.
`ModuleContract.model_validate(_module_payload()).guide_url is None` pins the
field itself. `test_module_contract_round_trips_guide_url` is not vacuous,
since `extra="forbid"` rejects an undeclared field, but its name promises a
round-trip it does not perform.
**`AGENTS.md:468` "Nothing declares that link" is not quite right.**
`provider.yaml` declares `integrations[].how-to-guide` and
`transfers[].how-to-guide`, and `check_doc_files` in
`run_provider_yaml_files_check.py` set-compares that declared mapping against a
glob of the operator, sensor and transfer guide paths, so it fails CI rather
than rotting silently. The justification that does hold is the one worth
writing down: the declared field names a page and never a section, and it does
not reach the toolset, hook or decorator pages this reads. Worth correcting
because `AGENTS.md` is a durable instruction file.
Two nits. `.provider-detail-page .module-actions` in `main.css` is a flex
row with no `flex-wrap`, and this PR takes it from two links to three, so
`flex-wrap: wrap` is cheap insurance at narrow viewports or a raised root font
size. And `git_ls_tree` in `extract_versions.py` runs with `core.quotePath` at
its default, which C-quotes a non-ASCII path so it fails the
`.endswith(".rst")` test while the working-tree reader accepts it; `-c
core.quotePath=false` closes that divergence. No such path exists under
`providers/` today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
