From ed5954d010aae381abe82afc71ff42a04d108efc Mon Sep 17 00:00:00 2001 From: Polichinl Date: Sat, 27 Jun 2026 02:15:35 +0200 Subject: [PATCH] fix(manager): log-before-raise on the 3 remaining unfao.py raises (#13, C-19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-008 requires structural failures to be logged persistently AND raised. Three manager raises still raised without a preceding logger.error: the missing-ensemble and missing-loa guards in _read_forecast_data, and the datasets-None guard in _save. Apply the same construct-msg -> logger.error -> raise pattern the _validate gates already use. Also updates the UNFAOPostProcessorManager CIC §3 to match (drops the "partially compliant / 3 raises lacking" qualifier added in #66; #13 resolved). ruff clean; 126 passed (the unrelated CI-skipped datafactory stub deselected). Co-Authored-By: Claude Opus 4.8 --- docs/CICs/UNFAOPostProcessorManager.md | 2 +- views_postprocessing/unfao/managers/unfao.py | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/CICs/UNFAOPostProcessorManager.md b/docs/CICs/UNFAOPostProcessorManager.md index b4d6b29..568bbaa 100644 --- a/docs/CICs/UNFAOPostProcessorManager.md +++ b/docs/CICs/UNFAOPostProcessorManager.md @@ -36,7 +36,7 @@ It is the single entrypoint for producing and delivering UN FAO-formatted predic - Guarantees that geographic metadata is added via `GaulLookupEnricher.enrich_dataframe_with_pg_info()` (a cell-id merge against the precomputed lookup) - Guarantees that required metadata columns are validated before upload - Guarantees that both historical and forecast datasets are uploaded to the UN FAO Appwrite bucket with correct metadata (name, loa, type, category) -- **Partially** logs-and-raises structural failures (ADR-008): the `_validate` gates and the S0–S6 delivery guards comply, but 3 raises in `unfao.py` (missing `ensemble`, missing `loa`, datasets-None in `_save`) still raise without a preceding `logger.error` — tracked by **C-19 / #13** +- Logs structural failures before raising them (ADR-008): the config/`loa` guards, the `_validate` gates, and the dataset/`_save` guard all `logger.error`-then-raise (#13 / C-19 resolved); the `delivery/` invariants raise representation-free, with the manager logging context at each call site --- diff --git a/views_postprocessing/unfao/managers/unfao.py b/views_postprocessing/unfao/managers/unfao.py index 8e1e60b..08ecaee 100644 --- a/views_postprocessing/unfao/managers/unfao.py +++ b/views_postprocessing/unfao/managers/unfao.py @@ -67,7 +67,9 @@ def _read_forecast_data(self): # Forecast Data ensemble_name = self.configs.get("ensemble", None) if not ensemble_name: - raise ValueError("Ensemble name must be provided in configs with the `ensemble` key for forecasting. Cannot proceed.") + err_msg = "Ensemble name must be provided in configs with the `ensemble` key for forecasting. Cannot proceed." + logger.error(err_msg) + raise ValueError(err_msg) self.ensemble_path_manager = EnsemblePathManager(ensemble_name_or_path=ensemble_name, validate=False) # ensemble_configs = EnsembleManager( @@ -77,7 +79,9 @@ def _read_forecast_data(self): # loa = ensemble_configs.get("level", None) loa = "pgm" if not loa: - raise ValueError("level must be defined in the ensemble configurations (e.g, pgm, cm). Cannot proceed.") + err_msg = "level must be defined in the ensemble configurations (e.g, pgm, cm). Cannot proceed." + logger.error(err_msg) + raise ValueError(err_msg) # Force it to the correct .env just to be safe load_dotenv(dotenv_path=str(self.ensemble_path_manager.dotenv)) @@ -284,7 +288,9 @@ def _check_coverage(self) -> None: def _save(self) -> list: if self._historical_dataset is None or self._forecast_dataset is None: - raise ValueError("Datasets could not be initialized properly.") + err_msg = "Datasets could not be initialized properly." + logger.error(err_msg) + raise ValueError(err_msg) # self._historical_dataset.dataframe.to_parquet( # self._model_path.data_generated / "historical_dataset.parquet"