[PR] Fix registry incremental extract dropping modules from catalog [airflow]

["kaxil (via GitHub)" <gi...@apache.org>]

kaxil opened a new pull request, #65975:
URL: https://github.com/apache/airflow/pull/65975

   ## Summary
   
   Two related bugs in `dev/registry/extract_parameters.py` and `dev/registry/merge_registry_data.py` that combined to silently corrupt the global module catalog on every single-provider incremental run, plus a multi-provider input contract that's currently a lie.
   
   ### Bug 1 — `--provider` doesn't accept space-separated input
   
   `extract_metadata.py` ([line 377](https://github.com/apache/airflow/blob/main/dev/registry/extract_metadata.py#L377)) and `extract_connections.py` ([line 218](https://github.com/apache/airflow/blob/main/dev/registry/extract_connections.py#L218)) both accept space-separated provider IDs. `extract_parameters.py` treats the whole string as a single ID and errors out. The `registry-build.yml` workflow_dispatch input documents `provider` as "Provider ID(s) for incremental build (space-separated, empty = full build)" — a multi-provider dispatch fails with `"provider 'amazon google ...' not found in provider.yaml files"`.
   
   ### Bug 2 — `modules.json` wipe on incremental
   
   In `--provider X` mode, `extract_parameters.py` skipped writing `modules.json` entirely on the assumption that "the output would be incomplete and would clobber the full modules.json from a previous build". `merge_registry_data.py` then drove module drops off `new_providers`'s IDs:
   
   ```python
   new_ids = {p["id"] for p in new_providers}
   merged_modules = [m for m in existing_modules if m["provider_id"] not in new_ids]
   merged_modules.extend(new_modules)  # often empty
   ```
   
   Net effect: for every provider listed in `new_providers`, the merge removed that provider's module entries from the global catalog and replaced them with nothing. Every successful single-provider auto-trigger from `publish-docs-to-s3.yml`'s `update-registry` job silently removed the targeted provider's modules (e.g. all of amazon's 377 entries) from the catalog used by `/explore/`, the per-provider operator listings, and PageFind search. A subsequent full build restored everything; in between, the registry was degraded.
   
   ## Design rationale
   
   **Why always write `modules.json` (even partial in `--provider` mode)?** The merge step is the right place to enforce "preserve non-targeted providers." Making the extractor responsible for "do I clobber?" coupled extraction state to merge state. The fix decouples them: extractor always writes what it discovered (full or partial); merge replaces only the providers actually present in the new payload.
   
   **Why drive merge drops off `new_modules` instead of `new_providers`?** It's the conservative invariant: if you have replacement modules, replace; if you don't, leave existing alone. The previous logic assumed `new_providers` and `new_modules` track each other perfectly, which is wrong when extraction skips writes (or, post-fix, when a provider has no module data to write).
   
   **Trade-off**: a provider that genuinely went from N modules to 0 between releases (refactored away, deprecated to config-only) leaves stale entries in the catalog until a full build clears them. That's preferable to the prior behaviour where every single-provider incremental update silently wiped the targeted provider's modules. Comment added in `merge_registry_data.py` documenting the trade-off.
   
   **Sibling-extractor parity**: `extract_metadata.py` and `extract_connections.py` already split `--provider` on whitespace. The new `_parse_requested_providers` helper mirrors that pattern.
   
   ## Gotchas
   
   - `runtime_modules.json` (debug stats only) is still skipped in `--provider` mode — partial stats would be misleading. No production code reads this file.
   - The new `--provider "  "` (whitespace-only) case parses to an empty set, which is falsy, so downstream `if requested_providers` skips the filter — same as the sibling scripts. Probably an accidental full rebuild if a CI variable is unset, but matches existing behaviour.
   
   ## Known follow-ups
   
   - Multi-provider dispatch via `registry-build.yml`'s `workflow_dispatch` is still not exercised end-to-end in CI. That's a separate orchestration question, not in scope here.
   - The single-line `print(f"Wrote ... modules ({scope_label}) ...")` log was hoisted out of the loop body since `scope_label` is loop-invariant.
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix registry incremental extract dropping modules from catalog [airflow]

["shahar1 (via GitHub)" <gi...@apache.org>]

shahar1 commented on PR #65975:
URL: https://github.com/apache/airflow/pull/65975#issuecomment-4330206692

   Encountered this issue [here](https://github.com/apache/airflow/actions/runs/25014739128/job/73265227706), you preceeded me with a fix :)
   
   https://github.com/apache/airflow/pull/65976


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix registry incremental extract dropping modules from catalog [airflow]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk merged PR #65975:
URL: https://github.com/apache/airflow/pull/65975


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org