Merge pull request #983 from jeremyh/develop

Fix exception in `datacube dataset update` for numeric keys
opendatacube · Jul 6, 2020 · 8152f8c · 8152f8c
2 parents dae57fb + 8691ebc
commit 8152f8c
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 20 deletions.
diff --git a/datacube/index/_datasets.py b/datacube/index/_datasets.py
@@ -10,7 +10,7 @@
 
 from datacube.model import Dataset, DatasetType
 from datacube.model.utils import flatten_datasets
-from datacube.utils import jsonify_document, changes, cached_property
+from datacube.utils import jsonify_document, _readable_offset, changes, cached_property
 from datacube.utils.changes import get_doc_changes
 from . import fields
 
@@ -265,7 +265,7 @@ def can_update(self, dataset, updates_allowed=None):
 
         return not bad_changes, good_changes, bad_changes
 
-    def update(self, dataset, updates_allowed=None):
+    def update(self, dataset: Dataset, updates_allowed=None):
         """
         Update dataset metadata and location
         :param Dataset dataset: Dataset to update
@@ -280,17 +280,21 @@ def update(self, dataset, updates_allowed=None):
             _LOG.info("No changes detected for dataset %s", dataset.id)
             return dataset
 
-        if not can_update:
-            full_message = "Unsafe changes at " + ", ".join(".".join(offset) for offset, _, _ in unsafe_changes)
-            raise ValueError(full_message)
-
-        _LOG.info("Updating dataset %s", dataset.id)
-
         for offset, old_val, new_val in safe_changes:
-            _LOG.info("Safe change from %r to %r", old_val, new_val)
+            _LOG.info("Safe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
 
         for offset, old_val, new_val in unsafe_changes:
-            _LOG.info("Unsafe change from %r to %r", old_val, new_val)
+            _LOG.warning("Unsafe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
+
+        if not can_update:
+            raise ValueError(f"Unsafe changes in {dataset.id}: " + (
+                ", ".join(
+                    _readable_offset(offset)
+                    for offset, _, _ in unsafe_changes
+                )
+            ))
+
+        _LOG.info("Updating dataset %s", dataset.id)
 
         product = self.types.get_by_name(dataset.type.name)
         with self._db.begin() as transaction:

diff --git a/datacube/index/_metadata_types.py b/datacube/index/_metadata_types.py
@@ -111,11 +111,11 @@ def can_update(self, metadata_type, allow_unsafe_updates=False):
             _LOG.info("Safe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
 
         for offset, old_val, new_val in bad_changes:
-            _LOG.info("Unsafe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
+            _LOG.warning("Unsafe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
 
         return allow_unsafe_updates or not bad_changes, good_changes, bad_changes
 
-    def update(self, metadata_type, allow_unsafe_updates=False, allow_table_lock=False):
+    def update(self, metadata_type: MetadataType, allow_unsafe_updates=False, allow_table_lock=False):
         """
         Update a metadata type from the document. Unsafe changes will throw a ValueError by default.
 
@@ -137,9 +137,12 @@ def update(self, metadata_type, allow_unsafe_updates=False, allow_table_lock=Fal
             return self.get_by_name(metadata_type.name)
 
         if not can_update:
-            full_message = "Unsafe changes at " + ", ".join(".".join(map(str, offset))
-                                                            for offset, _, _ in unsafe_changes)
-            raise ValueError(full_message)
+            raise ValueError(f"Unsafe changes in {metadata_type.name}: " + (
+                ", ".join(
+                    _readable_offset(offset)
+                    for offset, _, _ in unsafe_changes
+                )
+            ))
 
         _LOG.info("Updating metadata type %s", metadata_type.name)
 

diff --git a/datacube/index/_products.py b/datacube/index/_products.py
@@ -148,11 +148,11 @@ def can_update(self, product, allow_unsafe_updates=False):
             _LOG.info("Safe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
 
         for offset, old_val, new_val in bad_changes:
-            _LOG.info("Unsafe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
+            _LOG.warning("Unsafe change in %s from %r to %r", _readable_offset(offset), old_val, new_val)
 
         return allow_unsafe_updates or not bad_changes, good_changes, bad_changes
 
-    def update(self, product, allow_unsafe_updates=False, allow_table_lock=False):
+    def update(self, product: DatasetType, allow_unsafe_updates=False, allow_table_lock=False):
         """
         Update a product. Unsafe changes will throw a ValueError by default.
 
@@ -176,13 +176,12 @@ def update(self, product, allow_unsafe_updates=False, allow_table_lock=False):
             return self.get_by_name(product.name)
 
         if not can_update:
-            full_message = "Unsafe changes at " + (
+            raise ValueError(f"Unsafe changes in {product.name}: " + (
                 ", ".join(
                     _readable_offset(offset)
                     for offset, _, _ in unsafe_changes
                 )
-            )
-            raise ValueError(full_message)
+            ))
 
         _LOG.info("Updating product %s", product.name)