Merge pull request #195 from man-group/fix-gridfs-deletion

Fix GridFS item deletion

notebooker/serialization/mongo.py

@@ -524,15 +524,14 @@ def n_all_results_for_report_name(self, report_name: str) -> int:
     def delete_result(self, job_id: AnyStr) -> Dict[str, Any]:
         result = self._get_raw_check_result(job_id)
         status = JobStatus.from_string(result["status"])
-        gridfs_filenames = []
-        if status == JobStatus.DONE:
-            gridfs_filenames = load_files_from_gridfs(self.result_data_store, result, do_read=False)
-        elif status in (JobStatus.ERROR, JobStatus.TIMEOUT, JobStatus.CANCELLED):
-            gridfs_filenames = [_error_info_filename(job_id)]
+        gridfs_filenames = load_files_from_gridfs(self.result_data_store, result, do_read=False)
+        if status in (JobStatus.ERROR, JobStatus.TIMEOUT, JobStatus.CANCELLED):
+            gridfs_filenames.append(_error_info_filename(job_id))
         self.update_check_status(job_id, JobStatus.DELETED)
         for filename in gridfs_filenames:
             logger.info(f"Deleting {filename}")
-            self.result_data_store.delete(filename)
+            for grid_out in self.result_data_store.find({"filename": filename}):
+                self.result_data_store.delete(grid_out._id)
         return {"deleted_result_document": result, "gridfs_filenames": gridfs_filenames}
 
 

pyproject.toml

@@ -1,3 +1,6 @@
 [tool.black]
 line-length = 120
-target-version = ['py36', 'py37', 'py38']
+target-version = ['py36', 'py37', 'py38']
+
+[tool.ruff]
+line-length = 120

tests/integration/test_mongo.py

@@ -2,7 +2,6 @@
 import uuid
 
 import pytest
-from gridfs.errors import NoFile
 
 from notebooker.constants import JobStatus, NotebookResultComplete, NotebookResultError
 from notebooker.serialization.serialization import initialize_serializer_from_config
@@ -83,9 +82,9 @@ def test_delete(bson_library, webapp_config):
     assert result is not None
     assert result.raw_html == raw_html
     deleted_stuff = serializer.delete_result(job_id)
+    assert [r for r in serializer.result_data_store.list()] == []
     for filename in deleted_stuff["gridfs_filenames"]:
-        with pytest.raises(NoFile):
-            serializer.result_data_store.get(filename)
+        assert serializer.result_data_store.exists(filename=filename) is False
     assert serializer.get_check_result(job_id) is None
 
 
@@ -111,7 +110,7 @@ def test_delete_error(bson_library, webapp_config, job_status):
     result = serializer.get_check_result(job_id)
     assert result is not None
     deleted_stuff = serializer.delete_result(job_id)
+    assert [r for r in serializer.result_data_store.list()] == []
     for filename in deleted_stuff["gridfs_filenames"]:
-        with pytest.raises(NoFile):
-            serializer.result_data_store.get(filename)
+        assert serializer.result_data_store.exists(filename=filename) is False
     assert serializer.get_check_result(job_id) is None