Merge remote-tracking branch 'upstream/main' into fl-poc-with-main-me…

…rged

cli/medperf/commands/dataset/prepare.py

@@ -260,6 +260,13 @@ def run_statistics(self):
             self.dataset.unmark_as_ready()
             raise e
 
+        with open(self.out_statistics_path) as f:
+            stats = yaml.safe_load(f)
+
+        if stats is None:
+            self.dataset.unmark_as_ready()
+            raise ExecutionError("Statistics file is empty.")
+
         self.ui.print("> Statistics complete")
 
     def mark_dataset_as_ready(self):

cli/medperf/tests/commands/dataset/test_prepare.py

@@ -56,7 +56,7 @@ def test_get_prep_cube_downloads_cube_file(mocker, data_preparation, cube):
 
 @pytest.mark.parametrize("dataset_for_test", [False, True])
 def test_prepare_with_test_data_doesnt_send_reports(
-        mocker, data_preparation, dataset_for_test, cube, comms, fs
+    mocker, data_preparation, dataset_for_test, cube, comms, fs
 ):
     # Arrange
     data_preparation.dataset.for_test = dataset_for_test
@@ -86,7 +86,9 @@ def test_prepare_runs_then_stops_report_handler(
     mocker.patch.object(data_preparation.dataset, "write")
     mocked_obs = mocker.create_autospec(spec=Observer)
     mocker.patch(PATCH_REGISTER.format("Observer"), side_effect=mocked_obs)
-    gen_report_spy = mocker.patch(PATCH_REGISTER.format("DataPreparation._DataPreparation__generate_report_dict"))
+    gen_report_spy = mocker.patch(
+        PATCH_REGISTER.format("DataPreparation._DataPreparation__generate_report_dict")
+    )
 
     # Act
     data_preparation.run_prepare()
@@ -208,11 +210,14 @@ def _failure_run(*args, **kwargs):
 
 @pytest.mark.parametrize("metadata_specified", [False, True])
 def test_statistics_checks_metadata_path(
-    mocker, data_preparation, metadata_specified, cube
+    mocker, data_preparation, metadata_specified, cube, fs
 ):
     # Arrange
     spy = mocker.patch.object(cube, "run")
     data_preparation.metadata_specified = metadata_specified
+    data_preparation.out_statistics_path = "test.yaml"
+    fs.create_file(data_preparation.out_statistics_path, contents="")
+    mocker.patch("yaml.safe_load", return_value={})
 
     # Act
     data_preparation.run_statistics()
@@ -224,6 +229,23 @@ def test_statistics_checks_metadata_path(
         assert "metadata_path" not in spy.call_args.kwargs.keys()
 
 
+def test_preparation_fails_if_statistics_is_none(mocker, data_preparation, cube, fs):
+
+    # Arrange
+    unmark_spy = mocker.patch.object(data_preparation.dataset, "unmark_as_ready")
+    mocker.patch.object(cube, "run")
+    data_preparation.out_statistics_path = "test.yaml"
+    fs.create_file(data_preparation.out_statistics_path, contents="")
+    mocker.patch("yaml.safe_load", return_value=None)
+
+    # Act
+    with pytest.raises(ExecutionError):
+        data_preparation.run_statistics()
+
+    # Assert
+    unmark_spy.assert_called_once()
+
+
 def test_dataset_is_updated_after_report_sending(mocker, data_preparation, comms):
     # Arrange
     send_spy = mocker.patch.object(comms, "update_dataset")

server/dataset/migrations/0005_alter_dataset_generated_metadata.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.2.11 on 2024-11-29 16:23
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("dataset", "0004_auto_20231211_1827"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="dataset",
+            name="generated_metadata",
+            field=models.JSONField(blank=True, default=dict),
+        ),
+    ]

server/dataset/models.py

@@ -27,7 +27,7 @@ class Dataset(models.Model):
     state = models.CharField(
         choices=DATASET_STATE, max_length=100, default="DEVELOPMENT"
     )
-    generated_metadata = models.JSONField(default=dict, blank=True, null=True)
+    generated_metadata = models.JSONField(default=dict, blank=True)
     user_metadata = models.JSONField(default=dict, blank=True, null=True)
     report = models.JSONField(default=dict, blank=True, null=True)
     created_at = models.DateTimeField(auto_now_add=True)

server/dataset/serializers.py

@@ -29,7 +29,7 @@ class Meta:
         fields = "__all__"
         read_only_fields = ["owner"]
 
-    def validate_state(self, state):
+    def _validate_guid(self, data):
         # NOTE: this is checking the uniqueness of generated_uid across
         #       operational datasets. This check relies on the fact that
         #       such a constraint can only be violated by updating the state.
@@ -39,27 +39,38 @@ def validate_state(self, state):
         #       defined in models.py. The reason is that DRF doesn't translate
         #       uniqueness constraint correctly, causing a 500 server error
         #       if the check was left to be done by the database.
-        if state == "OPERATION" and self.instance.state == "DEVELOPMENT":
+        if (
+            data.get("state")
+            and data["state"] == "OPERATION"
+            and self.instance.state == "DEVELOPMENT"
+        ):
             constraint = (
                 Dataset.objects.all()
-                .filter(state="OPERATION", generated_uid=self.instance.generated_uid)
+                .filter(
+                    state="OPERATION",
+                    generated_uid=data.get(
+                        "generated_uid", self.instance.generated_uid
+                    ),
+                )
                 .exists()
             )
             if constraint:
                 raise serializers.ValidationError(
-                    "An Operational dataset with the same generated UID already exists"
+                    "An Operational dataset with the same "
+                    "generated UID already exists"
                 )
-        return state
 
     def validate(self, data):
         if self.instance.state == "OPERATION":
             editable_fields = ["is_valid", "user_metadata"]
             for k, v in data.items():
-                if k not in editable_fields:
-                    if v != getattr(self.instance, k):
-                        raise serializers.ValidationError(
-                            "User cannot update non editable fields in Operation mode"
-                        )
+                value_changed = v != getattr(self.instance, k)
+                if k not in editable_fields and value_changed:
+                    raise serializers.ValidationError(
+                        "User cannot update non editable fields in Operation mode"
+                    )
+        self._validate_guid(data)
+
         return data