python(feat): Allow adding TDMS metadata to existing runs (#320)

Author: marc-sift

python/lib/sift_py/data_import/_tdms_test.py

@@ -777,8 +777,8 @@ def post_side_effect(*args, **kwargs):
 
     svc = TdmsUploadService(rest_config)
 
-    # Should raise if run_id is provided
-    with pytest.raises(ValueError, match="Metadata can only be included in new runs"):
+    # Should raise if run_id and run_name is provided
+    with pytest.raises(ValueError, match="Must specify either run_name or run_id, not both"):
         svc.upload(
             "some_tdms.tdms",
             "asset_name",
@@ -788,7 +788,7 @@ def post_side_effect(*args, **kwargs):
         )
 
     # Should raise if run_name is not provided
-    with pytest.raises(ValueError, match="Must provide a run_name to include metadata"):
+    with pytest.raises(ValueError, match="Metadata can only be included in Runs"):
         svc.upload(
             "some_tdms.tdms",
             "asset_name",
@@ -845,3 +845,45 @@ def post_side_effect(*args, **kwargs):
         == MetadataKeyType.METADATA_KEY_TYPE_STRING
     )
     assert create_run_post_data["metadata"][4]["string_value"].startswith("2024-01-01T12:00:00")
+
+
+def test_tdms_upload_service_upload_with_metadata_run_id(
+    mocker: MockFixture, mock_waveform_tdms_file: MockTdmsFile
+):
+    mock_path_is_file = mocker.patch("sift_py.data_import.tdms.Path.is_file")
+    mock_path_is_file.return_value = True
+
+    mock_path_getsize = mocker.patch("sift_py.data_import.csv.os.path.getsize")
+    mock_path_getsize.return_value = 10
+
+    # Patch TdmsFile to return our mock file
+    mocker.patch("sift_py.data_import.tdms.TdmsFile", return_value=mock_waveform_tdms_file)
+
+    # Patch requests.Session.post and patch requests.Session.patch for metadata update
+    mock_requests_post = mocker.patch("sift_py.rest.requests.Session.post")
+    mock_requests_post.return_value = MockResponse()
+    mock_requests_patch = mocker.patch("sift_py.rest.requests.Session.patch")
+    mock_requests_patch.return_value = MockResponse(
+        status_code=200,
+        text=json.dumps({"run": {"runId": "existing_run_id"}}),
+    )
+
+    svc = TdmsUploadService(rest_config)
+
+    # Should succeed and call _add_metadata_to_run via PATCH with metadata if only run_id is provided
+    svc.upload(
+        "some_tdms.tdms",
+        "asset_name",
+        include_metadata=True,
+        run_id="existing_run_id",
+    )
+
+    # Check that PATCH was called for metadata update
+    patch_call = mock_requests_patch.call_args_list[0]
+    patch_data = json.loads(patch_call.kwargs["data"])
+    assert patch_data["run"]["runId"] == "existing_run_id"
+    assert "metadata" in patch_data["run"]
+    assert patch_data["updateMask"] == "metadata"
+    # Metadata keys should match those in the mock_tdms_file properties
+    keys = [md["key"]["name"] for md in patch_data["run"]["metadata"]]
+    assert set(keys) == set(mock_waveform_tdms_file.properties.keys())

python/lib/sift_py/data_import/csv.py

@@ -305,6 +305,36 @@ def _create_run(self, run_name: str, metadata: Optional[List[MetadataValue]] = N
 
         return run_info["run"]["runId"]
 
+    def _add_metadata_to_run(self, run_id: str, metadata: List[MetadataValue]):
+        """
+        Updates metadata for the specified Run.
+
+        Args:
+            run_id: The ID of the run to update.
+            metadata: Metadata fields to update.
+        """
+        run_uri = urljoin(self._base_uri, self.RUN_PATH)
+
+        req: Dict[str, Any] = {
+            "run": {
+                "runId": run_id,
+                "metadata": metadata_pb_to_dict_api(metadata),
+            },
+            "updateMask": "metadata",
+        }
+
+        response = self._session.patch(
+            url=run_uri,
+            headers={
+                "Content-Type": "application/json",
+            },
+            data=json.dumps(req),
+        )
+        if response.status_code != 200:
+            raise Exception(
+                f"Run metadata update failed with status code {response.status_code}. {response.text}"
+            )
+
 
 class _ProgressFile:
     """Displays the status with alive_bar while reading the file."""

python/lib/sift_py/data_import/tdms.py

@@ -147,15 +147,14 @@ def upload(
         if not posix_path.is_file():
             raise Exception(f"Provided path, '{path}', does not point to a regular file.")
 
-        # If metadata should be included, create the run first.
         if include_metadata:
             # Do not allow including metadata in existing runs since it could lead
             # to overwriting metadata fields.
-            if run_id:
-                raise ValueError("Metadata can only be included in new runs")
+            if not (run_id or run_name):
+                raise ValueError("Metadata can only be included in Runs")
 
-            if not run_name:
-                raise ValueError("Must provide a run_name to include metadata")
+            if run_name and run_id:
+                raise ValueError("Must specify either run_name or run_id, not both")
 
             def parse_datetime(value):
                 """Convert datetime metadata to strings."""
@@ -168,9 +167,15 @@ def parse_datetime(value):
 
             tdms_file = TdmsFile(path)
             metadata = metadata_dict_to_pb(tdms_file.properties, parse_datetime)
-            run_id = self._csv_upload_service._create_run(run_name, metadata)
-            # Clear the run name since we are using run_id now.
-            run_name = None
+
+            # Create a new run with metadata fields.
+            if run_name:
+                run_id = self._csv_upload_service._create_run(run_name, metadata)
+                # Clear the run name since we are using run_id now.
+                run_name = None
+            # Add metadata to existing Run.
+            else:
+                self._csv_upload_service._add_metadata_to_run(run_id, metadata)  # type: ignore
 
         with NamedTemporaryFile(mode="wt", suffix=".csv.gz") as temp_file:
             csv_config = self._convert_to_csv(