chore: migrate duckdb.connect() → lab_connectors.duckdb.safe_connect()

[Gabrymi93]

Migra 8 call site in 2 file da duckdb.connect() + try/finally + con.close() a with safe_connect() as con:.

• -22 righe nette
• Fixa connection leak in build_catalog_inventory.py (mancava try/finally)
• 84 test non-MCP passano invariati (test MCP falliscono per ragioni pre-esistenti)

[Gabrymi93]

mergiare prima dataciviclab/toolkit#240
per dipendenze toolkit

[Gabrymi93]

Review: chore: migrate duckdb.connect() → lab_connectors.duckdb.safe_connect() #226

Modalità: lite
Verdict: REQUEST CHANGES (fix-required)
Tipo: fix-required — direzione buona ma bug critico

Blocker

1. import requests rimosso ma requests ancora usato nel codice

• Impatto: NameError: name 'requests' is not defined a runtime in _download_public_to_temp() e _copy_gcs_to_temp().
• Evidenza: mcp/so_server_core.py — il diff rimuove import requests (linea 23) ma:
  • Linea 400: response = requests.get(_public_url(uri), timeout=120, stream=True)
  • Linea 416: except requests.RequestException:
• Fix: ripristinare import requests nella sezione import. Non fa parte del perimetro della PR (che è duckdb → safe_connect) e la rimozione è accidentale.

Warning

2. Rimozione import requests fuori scope

• La rimozione di import requests non è dichiarata né nel titolo né nella descrizione della PR. Il perimetro dichiarato è "migrate duckdb.connect() → safe_connect()". Modifiche collaterali non dichiarate vanno evitate o documentate.

Gate passati (tutto il resto è OK)

• Root-cause gate: la migrazione a safe_connect è corretta e allinea al contratto condiviso lab_connectors[duckdb]@v0.4.0
• 7 call site in so_server_core.py migrati correttamente (tutti con try/finally → with safe_connect())
• 1 call site in build_catalog_inventory.py migrato correttamente, fissa connection leak (mancava try/finally)
• import duckdb residuo giustificato (type annotation duckdb.DuckDBPyConnection)
• requirements.txt: bump a lab-connectors[duckdb]@v0.4.0 corretto e coerente
• requirements.txt: toolkit commit bump a a06005c (merge di toolkit #240 duckdb migration) corretto
• −22 righe nette, riduzione complessità
• Test non-MCP (84) passano invariati
• Unico duckdb.connect() residuo è in tests/test_so_mcp.py (test, fuori scope)

Rischi residui (post-fix)

Nessuno. Una volta ripristinato import requests, la PR è pulita.

Verdetto

La migrazione duckdb → safe_connect è corretta in tutti gli 8 call site. C'è un errore accidentale (rimozione import requests ancora necessario) che va corretto prima del merge.

Next owner: author
Next action: ripristinare import requests in mcp/so_server_core.py, poi merge

[Gabrymi93]

fixato in 4fa3783 mentre facevo fare review