feat: sync datasets_in_use da DI catalog nel radar daily#228
Merged
Conversation
- scripts/sync_datasets_in_use.py: sync manuale datasets_in_use da DI clean_catalog.json - radar.yml: step automatico dopo radar check daily - ci.yml: rimosso check (troppo frequente, meglio radar) - requirements.txt: aggiunto ruamel.yaml per roundtrip YAML
Gabrymi93
commented
May 13, 2026
Member
Author
Gabrymi93
left a comment
There was a problem hiding this comment.
Review: feat: sync datasets_in_use da DI catalog nel radar daily #228
Modalità: lite
Verdict: APPROVE
Gate passati
- Root-cause gate: La PR risolve il problema reale (mantenere
datasets_in_useallineato con DI era manuale). Non è un workaround — automatizza un sync che prima era operazione manuale. -
sync_datasets_in_use.py: logica lineare — fetch DI catalog viaurlopen(stdlib), raggruppa persource_id, roundtrip YAML conruamel.yaml. Idempotente: sesource_idnon presente, il dataset viene skipato. -
radar.yml: step sync POSIZIONATO correttamente — doporadar_check.py, prima del commit.sources_registry.yamlè incluso sia inupload-artifactche ingit add. Commit atomico con radar data. -
requirements.txt:ruamel.yaml>=0.18.10,<1.0aggiunto. Libreria standard per roundtrip YAML. Già installata nel workflow radar viapip install -r requirements.txt. - Cross-repo: dipende da DI PR #285 (source_id). Idempotente se #285 non è mergiata —
ds.get("source_id")restituisce None, dataset skipato, nessuna modifica. Sicuro da mergeare prima di #285. - Nessuna duplicazione: script autonomo, non duplica logica esistente.
- Nessun test richiesto: script operativo CI, logica banale (fetch + group + update YAML).
Warning
1. ci.yml: whitespace noise
- Due righe vuote aggiunte prima di
- name: Run tests. Nessun impatto funzionale, ma rumore nel diff. - Fix opzionale: rimuovere le righe extra.
2. fetch_di_catalog senza try/except
- Se il fetch fallisce (rete down, DI catalog inaccessibile), lo script lancia eccezione non gestita e fallisce con traceback.
- Accettabile: il fallimento è visibile nei log CI, e lo step non blocca altri step del workflow (successivi separatemente). Per script CI operativo è sufficiente.
Nota
urlopenvsrequests: lo script usaurllib.request.urlopen(stdlib) invece direquests(già in requirements.txt). Scelta voluta: zero dipendenze per un fetch singolo. Coerente con lo spirito "leggero" dello script.- Commit messaggio: il commit radar dice
chore(so): refresh radar status [ci skip]— non menziona datasets_in_use. Poiché il sync viene dopo radar_check e prima del commit, eventuali cambi asources_registry.yamlvengono inclusi automaticamente. Il messaggio è generico ma corretto.
Verdetto
PR solida che chiude un gap operativo (sync datasets_in_use manuale → automatico via radar daily). Script pulito, posizionamento corretto nel workflow, idempotente rispetto alla dipendenza upstream (DI #285).
Next owner: maintainer
Next action: merge (può essere mergiato prima o dopo DI #285 — idempotente in entrambi i casi)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cosa cambia
scripts/sync_datasets_in_use.py: leggeclean_catalog.jsonda dataset-incubator, raggruppa dataset persource_id, aggiornasources_registry.yaml.github/workflows/radar.yml: step che esegue il sync dopo il radar check daily.github/workflows/ci.yml: rimosso (radar e' il posto giusto per sync daily)requirements.txt: aggiuntoruamel.yamlper roundtrip YAMLCome funziona
Dipendenze
Richiede PR #285 di dataset-incubator (aggiunge source_id a clean_catalog.json).
Prima che quella PR sia mergiata, lo script non trova source_id e non fa nulla (idempotente).
Riferimenti