Delete snapshot after upload

Author: wk9874

simvue/api/objects/artifact/file.py

@@ -124,4 +124,8 @@ def new(
         with open(_file_orig_path, "rb") as out_f:
             _artifact._upload(file=out_f, timeout=upload_timeout, file_size=_file_size)
 
+        # If snapshot created, delete it after uploading
+        if pathlib.Path(_file_orig_path).parent == _artifact._local_staging_file.parent:
+            pathlib.Path(_file_orig_path).unlink()
+
         return _artifact

simvue/sender.py

@@ -86,39 +86,38 @@ def upload_cached_file(
     if issubclass(_instance_class, simvue.api.objects.ObjectArtifact):
         with open(file_path.parent.joinpath(f"{_current_id}.object"), "rb") as file:
             _data["serialized"] = file.read()
+    try:
+        # We want to reconnect if there is an online ID stored for this file
+        if _online_id := id_mapping.get(_current_id):
+            obj_for_upload = _instance_class(
+                identifier=_online_id, _read_only=False, **_data
+            )
+        else:
+            obj_for_upload = _instance_class.new(**_data)
 
-    # We want to reconnect if there is an online ID stored for this file
-    if _online_id := id_mapping.get(_current_id):
-        obj_for_upload = _instance_class(
-            identifier=_online_id, _read_only=False, **_data
-        )
-    else:
-        obj_for_upload = _instance_class.new(**_data)
-
-    with lock:
-        obj_for_upload.on_reconnect(id_mapping)
+        with lock:
+            obj_for_upload.on_reconnect(id_mapping)
 
-    try:
         if not issubclass(_instance_class, ArtifactBase):
             obj_for_upload.commit()
         _new_id = obj_for_upload.id
+
     except Exception as error:
         if "status 409" in error.args[0]:
             return
+
         _logger.error(
-            f"Error while committing '{obj_for_upload.__class__.__name__}': {error.args[0]}"
+            f"Error while committing '{_instance_class.__name__}': {error.args[0]}"
         )
         _log_upload_failed(file_path)
         return
     if not _new_id:
-        _logger.error(
-            f"Object of type '{obj_for_upload.__class__.__name__}' has no identifier"
-        )
+        _logger.error(f"Object of type '{_instance_class.__name__}' has no identifier")
         _log_upload_failed(file_path)
         return
 
     _logger.info(
-        f"{'Updated' if id_mapping.get(_current_id) else 'Created'} {obj_for_upload.__class__.__name__} '{_new_id}'"
+        f"{'Updated' if id_mapping.get(_current_id) else 'Created'} {_instance_class.__name__} '{_new_id}'"
     )
 
     file_path.unlink(missing_ok=True)

tests/unit/test_file_artifact.py

@@ -10,10 +10,15 @@
 from simvue.api.objects.folder import Folder
 from simvue.sender import sender
 from simvue.client import Client
+import logging
 
 @pytest.mark.api
 @pytest.mark.online
-def test_file_artifact_creation_online() -> None:
+@pytest.mark.parametrize(
+    "snapshot",
+    (True, False)
+)
+def test_file_artifact_creation_online(offline_cache_setup, snapshot) -> None:
     _uuid: str = f"{uuid.uuid4()}".split("-")[0]
     _folder_name = f"/simvue_unit_testing/{_uuid}"
     _folder = Folder.new(path=_folder_name)
@@ -32,7 +37,8 @@ def test_file_artifact_creation_online() -> None:
             file_path=_path,
             storage=None,
             mime_type=None,
-            metadata=None
+            metadata=None,
+            snapshot=snapshot
         )
         _artifact.attach_to_run(_run.id, "input")
         time.sleep(1)
@@ -45,6 +51,11 @@ def test_file_artifact_creation_online() -> None:
         _content = b"".join(_artifact.download_content()).decode("UTF-8")
         assert _content == f"Hello World! {_uuid}"
         assert _artifact.to_dict()
+        
+        # If snapshotting, check no local copy remains
+        if snapshot:
+            assert len(list(_artifact._local_staging_file.parent.iterdir())) == 0
+
     _run.delete()
     _folder.delete(recursive=True, delete_runs=True, runs_only=False)
     with contextlib.suppress(FileNotFoundError):
@@ -55,7 +66,11 @@ def test_file_artifact_creation_online() -> None:
 
 @pytest.mark.api
 @pytest.mark.offline
-def test_file_artifact_creation_offline(offline_cache_setup) -> None:
+@pytest.mark.parametrize(
+    "snapshot",
+    (True, False)
+)
+def test_file_artifact_creation_offline(offline_cache_setup, snapshot) -> None:
     _uuid: str = f"{uuid.uuid4()}".split("-")[0]
     _folder_name = f"/simvue_unit_testing/{_uuid}"
     _folder = Folder.new(path=_folder_name, offline=True)
@@ -74,7 +89,8 @@ def test_file_artifact_creation_offline(offline_cache_setup) -> None:
         storage=None,
         mime_type=None,
         offline=True,
-        metadata=None
+        metadata=None,
+        snapshot=snapshot
     )
     _artifact.attach_to_run(_run._identifier, category="input")
 
@@ -84,9 +100,15 @@ def test_file_artifact_creation_offline(offline_cache_setup) -> None:
     assert _local_data.get("name") == f"test_file_artifact_{_uuid}"
     assert _local_data.get("runs") == {_run._identifier: "input"}
 
+    # If snapshot, check artifact definition file and a copy of the actual file exist in staging area
+    assert len(list(_artifact._local_staging_file.parent.iterdir())) == 2 if snapshot else 1
+    
     _id_mapping = sender(pathlib.Path(offline_cache_setup.name), 1, 10)
     time.sleep(1)
 
+    # Check file(s) deleted after upload
+    assert len(list(_artifact._local_staging_file.parent.iterdir())) == 0
+    
     _online_artifact = Artifact(_id_mapping[_artifact.id])
     assert _online_artifact.name == _artifact.name
     _content = b"".join(_online_artifact.download_content()).decode("UTF-8")
@@ -101,11 +123,11 @@ def test_file_artifact_creation_offline(offline_cache_setup) -> None:
     "snapshot",
     (True, False)
 )
-def test_file_artifact_creation_offline_snapshot(offline_cache_setup, snapshot) -> None:
+def test_file_artifact_creation_offline_updated(offline_cache_setup, caplog, snapshot) -> None:
     _uuid: str = f"{uuid.uuid4()}".split("-")[0]
     _folder_name = f"/simvue_unit_testing/{_uuid}"
     _folder = Folder.new(path=_folder_name, offline=True)
-    _run = Run.new(name=f"test_file_artifact_creation_offline_snapshot_{_uuid}",folder=_folder_name, offline=True) 
+    _run = Run.new(name=f"test_file_artifact_creation_offline_updated_{_uuid}",folder=_folder_name, offline=True) 
 
     _path = pathlib.Path(offline_cache_setup.name).joinpath("hello_world.txt")
 
@@ -136,8 +158,9 @@ def test_file_artifact_creation_offline_snapshot(offline_cache_setup, snapshot)
         out_f.write("File changed!")
 
     if not snapshot:
-        with pytest.raises(RuntimeError): # TODO: Make sender be resilient to this?
+        with caplog.at_level(logging.ERROR): 
             _id_mapping = sender(pathlib.Path(offline_cache_setup.name), 1, 10)
+        assert "The SHA256 you specified did not match the calculated checksum." in caplog.text
         return
     else:
         _id_mapping = sender(pathlib.Path(offline_cache_setup.name), 1, 10)