Merge pull request #675 from Climate-REF/perf/skip-redundant-validate

perf(ingest): drop redundant per-dataset validate_data_catalog call

Author: lewisjared

changelog/675.improvement.md

@@ -0,0 +1,6 @@
+Skip the redundant per-dataset `validate_data_catalog` call inside `register_dataset`.
+The production ingest path already validates the catalog (and each streamed chunk) once
+up-front, so the inner re-validation only duplicated work. Cuts ~20% off ingest wall time
+on a 50k-file / 500-dataset synthetic CMIP6 archive. A cheap per-slice guard (no groupby)
+remains so callers that bypass the upstream validation contract still get a clear error
+instead of silently registering inconsistent metadata.

packages/climate-ref/conftest.py

@@ -84,16 +84,19 @@ def db_seeded_template(tmp_path_session, cmip6_data_catalog, obs4mips_data_catal
     database = Database(f"sqlite:///{template_db_path}")
     database.migrate(config)
 
-    # Seed the CMIP6 sample datasets
+    # Seed the CMIP6 sample datasets. ``register_dataset`` trusts callers to
+    # have already validated the catalog, so do that here before iterating.
     adapter = CMIP6DatasetAdapter()
+    cmip6_validated = adapter.validate_data_catalog(cmip6_data_catalog)
     with database.session.begin():
-        for instance_id, data_catalog_dataset in cmip6_data_catalog.groupby(adapter.slug_column):
+        for instance_id, data_catalog_dataset in cmip6_validated.groupby(adapter.slug_column):
             adapter.register_dataset(database, data_catalog_dataset)
 
     # Seed the obs4MIPs sample datasets
     adapter_obs = Obs4MIPsDatasetAdapter()
+    obs4mips_validated = adapter_obs.validate_data_catalog(obs4mips_data_catalog)
     with database.session.begin():
-        for instance_id, data_catalog_dataset in obs4mips_data_catalog.groupby(adapter_obs.slug_column):
+        for instance_id, data_catalog_dataset in obs4mips_validated.groupby(adapter_obs.slug_column):
             adapter_obs.register_dataset(database, data_catalog_dataset)
 
     with database.session.begin():

packages/climate-ref/src/climate_ref/datasets/base.py

@@ -204,26 +204,45 @@ def register_dataset(  # noqa: PLR0912, PLR0915
         """
         Register a dataset in the database using the data catalog
 
+        This assumes that the data catalog has already been validated with `validate_data_catalog`
+        to ensure that the dataset-specific metadata is consistent across all files in the dataset.
+
         Parameters
         ----------
         db
             Database instance
         data_catalog_dataset
             A subset of the data catalog containing the metadata for a single dataset
 
+
+        Raises
+        ------
+        RefException
+            If the data catalog contains validation errors that should have been caught by
+                `validate_data_catalog` (i.e. multiple unique slugs in `slug_column`).
+
+
         Returns
         -------
         :
             Registration result with dataset and file change information
         """
         DatasetModel = self.dataset_cls
 
-        self.validate_data_catalog(data_catalog_dataset)
         unique_slugs = data_catalog_dataset[self.slug_column].unique()
         if len(unique_slugs) != 1:
             raise RefException(f"Found multiple datasets in the same directory: {unique_slugs}")
         slug = unique_slugs[0]
 
+        # Callers are responsible for validating the catalog with  ``validate_data_catalog`` before invoking.
+        # This is a strict subset of ``validate_data_catalog`` to catch skipping upstream validation.
+        slice_meta = data_catalog_dataset[list(self.dataset_specific_metadata)]
+        if (slice_meta.nunique(dropna=False) > 1).any():
+            raise RefException(
+                f"Dataset {slug} has inconsistent dataset-specific metadata; "
+                "callers must pre-validate the catalog with validate_data_catalog."
+            )
+
         # Check if the incoming data is unfinalised (DRS parser) and the dataset
         # already exists as finalised.  In that case, skip the entire update to
         # avoid regressing metadata that was populated during finalisation.

packages/climate-ref/tests/unit/datasets/test_datasets.py

@@ -338,6 +338,33 @@ def test_register_dataset_multiple_datasets_error(monkeypatch, test_db):
             adapter.register_dataset(db=db, data_catalog_dataset=df)
 
 
+def test_register_dataset_inconsistent_metadata_raises(monkeypatch, test_db):
+    """Callers that bypass validate_data_catalog must still be protected from
+    silently registering a slice whose dataset-specific metadata varies."""
+    adapter, db = test_db
+
+    df = _mk_df(
+        rows=[
+            {
+                "path": "a.nc",
+                "start_time": "2001-01-01",
+                "end_time": "2001-12-31",
+                "experiment_id": "historical",
+            },
+            {
+                "path": "b.nc",
+                "start_time": "2002-01-01",
+                "end_time": "2002-12-31",
+                "experiment_id": "ssp585",
+            },
+        ]
+    )
+
+    with pytest.raises(RefException, match="inconsistent dataset-specific metadata"):
+        with db.session.begin():
+            adapter.register_dataset(db=db, data_catalog_dataset=df)
+
+
 def test_register_dataset_updates_dataset_metadata(monkeypatch, test_db):
     """Test that changes to dataset metadata are properly captured and result in UPDATED state"""
     adapter, db = test_db