fix: Auto-delete-input-file deleting input even in error case

.github/workflows/build.yml

@@ -10,7 +10,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
+        python-version: ["3.8", "3.9", "3.10", "3.11"]
         poetry-version: ["1.3.1"]
         os: [ubuntu-latest]
     runs-on: ${{ matrix.os }}

.github/workflows/pytest.yml

@@ -11,7 +11,7 @@ jobs:
       fail-fast: true
       max-parallel: 1
       matrix:
-        python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
+        python-version: ["3.8", "3.9", "3.10", "3.11"]
         poetry-version: ["1.3.1"]
         os: [ubuntu-latest]
     runs-on: ${{ matrix.os }}

Makefile

@@ -1,4 +1,4 @@
-VERSION=9.3.0
+VERSION=10.0.0
 
 SHELL := /bin/bash
 

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "redact"
-version = "9.3.0"
+version = "10.0.0"
 description = "Command-line and Python client for Brighter AI's Redact"
 authors = ["brighter AI <[email protected]>"]
 readme = "README.md"
@@ -11,7 +11,7 @@ redact_file = "redact.main:redact_file_entry_point"
 redact_folder = "redact.main:redact_folder_entry_point"
 
 [tool.poetry.dependencies]
-python = ">= 3.7, < 3.12"
+python = ">= 3.8, < 3.12"
 httpx = "^0.23.0"
 pydantic = "^1.9.2"
 tqdm = "^4.60.0"
@@ -21,9 +21,9 @@ StrEnum = "^0.4.9"
 
 [tool.poetry.dev-dependencies]
 pytest = "^7.1.2"
-Pillow = "^9.2.0"
+Pillow = "^10.0"
 uvicorn = "^0.18.2"
-fastapi = "^0.79.0"
+fastapi = "^0.115.6"
 pytest-timeout = "^2.1.0"
 isort = "^5.10.1"
 black = "^22.10.0"

redact/__init__.py

@@ -2,7 +2,7 @@
 Python client for "brighter Redact"
 """
 
-__version__ = "9.3.0"
+__version__ = "10.0.0"
 
 from .errors import RedactConnectError, RedactResponseError
 from .v4.data_models import (

redact/v3/tools/redact_file.py

@@ -125,21 +125,24 @@ def redact_file(
 
         # stream result to file
         job.download_result_to_file(file=output_path, ignore_warnings=ignore_warnings)
+
+        # write labels
+        if save_labels:
+            labels = job.get_labels().json()
+            with open(_get_labels_path(output_path), "w") as f:
+                f.write(labels)
+
+        # delete input file only if processing was successful
+        if auto_delete_input_file:
+            log.debug(f"Deleting {file_path}")
+            Path(file_path).unlink()
+
+        return job_status
+
     finally:
         if licence_plate_custom_stamp:
             licence_plate_custom_stamp.close()
 
-    # write labels
-    if save_labels:
-        labels = job.get_labels().json()
-        with open(_get_labels_path(output_path), "w") as f:
-            f.write(labels)
-
-    # delete input file
-    if auto_delete_input_file:
-        log.debug(f"Deleting {file_path}")
-        Path(file_path).unlink()
-
         try:
             # delete job
             if auto_delete_job:
@@ -149,8 +152,6 @@ def redact_file(
             # if the starting the job failed, there is no job variable and this Exception will be thrown
             pass
 
-    return job_status
-
 
 def _get_out_path(
     output_path: Union[str, Path], file_path: Path, output_type: OutputType

redact/v4/tools/redact_file.py

@@ -117,25 +117,28 @@ def redact_file(
 
         # stream result to file
         job.download_result_to_file(file=output_path, ignore_warnings=ignore_warnings)
-    finally:
-        if licence_plate_custom_stamp:
-            licence_plate_custom_stamp.close()
 
-        # delete input file
+        # delete input file only if processing was successful
         if auto_delete_input_file:
             log.debug(f"Deleting {file_path}")
             Path(file_path).unlink()
 
+        return job_status
+
+    finally:
+        if licence_plate_custom_stamp:
+            licence_plate_custom_stamp.close()
+
         try:
-            # delete job
+            # delete job in finally, to also delete and cancel server-side jobs if redact-client is killed (e.g. CTRL+C)
             if auto_delete_job:
                 log.debug(f"Deleting job {job.output_id}")
                 job.delete()
         except UnboundLocalError:
             # if the starting the job failed, there is no job variable and this Exception will be thrown
             pass
 
-    return job_status
+        # End of finally. Delete input file intentionally not included in finally.
 
 
 def _get_out_path(

tests/conftest.py

@@ -103,3 +103,9 @@ def image_path(tmp_path, resource_path):
 def video_path(tmp_path, resource_path):
     video_path = resource_path / "not_starting_with_keyframe.mp4"
     return _copy_file_to_tmp_path(tmp_path=tmp_path, file_path=video_path)
+
+
+@pytest.fixture
+def broken_video_path(tmp_path, resource_path):
+    video_path = resource_path / "broken_video.mp4"
+    return _copy_file_to_tmp_path(tmp_path=tmp_path, file_path=video_path)

tests/resources/broken_video.mp4

@@ -0,0 +1,3 @@
+broken
+video
+file

tests/v3/functional/test_redact_folder.py

@@ -113,3 +113,70 @@ def test_redact_folder_video_correct_file_ending_for_overlays(
         ]
         for file in files_in_out_dir:
             assert file.suffix == ".tar"
+
+    def test_redact_folder_auto_delete_input_only_on_success(
+        self, broken_video_path: Path, redact_url, optional_api_key, tmp_path_factory
+    ):
+        # GIVEN an input dir (with videos) and an output dir
+        videos_path = broken_video_path.parent
+        output_path = tmp_path_factory.mktemp("vid_dir_out")
+
+        files_in_input_dir_at_start = len([p for p in videos_path.rglob("*.*")])
+
+        redact_folder(
+            input_dir=videos_path,
+            output_dir=output_path,
+            input_type=InputType.videos,
+            output_type=OutputType.videos,
+            service=ServiceType.blur,
+            redact_url=redact_url,
+            api_key=optional_api_key,
+            n_parallel_jobs=1,
+            ignore_warnings=True,
+            auto_delete_input_file=True,
+        )
+
+        # THEN no input videos are anonymized in the output dir
+        files_in_out_dir = [
+            p.relative_to(output_path) for p in output_path.rglob("*.*")
+        ]
+        assert len(files_in_out_dir) == 0
+
+        # no input videos were deleted
+        files_in_input_dir = [
+            p.relative_to(videos_path) for p in videos_path.rglob("*.*")
+        ]
+        assert len(files_in_input_dir) == files_in_input_dir_at_start
+
+    def test_redact_folder_auto_delete_input_works(
+        self, images_path: Path, redact_url, optional_api_key, tmp_path_factory
+    ):
+        # GIVEN an input dir and an output dir
+        output_path = tmp_path_factory.mktemp("vid_dir_out")
+
+        files_in_input_dir_at_start = len([p for p in images_path.rglob("*.*")])
+
+        redact_folder(
+            input_dir=images_path,
+            output_dir=output_path,
+            input_type=InputType.images,
+            output_type=OutputType.images,
+            service=ServiceType.blur,
+            redact_url=redact_url,
+            api_key=optional_api_key,
+            n_parallel_jobs=3,
+            ignore_warnings=True,
+            auto_delete_input_file=True,
+        )
+
+        # THEN no input images are anonymized in the output dir
+        files_in_out_dir = [
+            p.relative_to(output_path) for p in output_path.rglob("*.*")
+        ]
+        assert len(files_in_out_dir) == files_in_input_dir_at_start
+
+        # all input images were deleted
+        files_in_input_dir = [
+            p.relative_to(images_path) for p in images_path.rglob("*.*")
+        ]
+        assert len(files_in_input_dir) == 0

tests/v4/functional/test_redact_folder.py

@@ -255,3 +255,70 @@ def test_redact_folder_with_areas_of_interest(
         ]
         for file in files_in_out_dir:
             assert file.suffix == ".jpeg"
+
+    def test_redact_folder_auto_delete_input_only_on_success(
+        self, broken_video_path: Path, redact_url, optional_api_key, tmp_path_factory
+    ):
+        # GIVEN an input dir (with videos) and an output dir
+        videos_path = broken_video_path.parent
+        output_path = tmp_path_factory.mktemp("vid_dir_out")
+
+        files_in_input_dir_at_start = len([p for p in videos_path.rglob("*.*")])
+
+        redact_folder(
+            input_dir=videos_path,
+            output_dir=output_path,
+            input_type=InputType.videos,
+            output_type=OutputType.videos,
+            service=ServiceType.blur,
+            redact_url=redact_url,
+            api_key=optional_api_key,
+            n_parallel_jobs=1,
+            ignore_warnings=True,
+            auto_delete_input_file=True,
+        )
+
+        # THEN no input videos are anonymized in the output dir
+        files_in_out_dir = [
+            p.relative_to(output_path) for p in output_path.rglob("*.*")
+        ]
+        assert len(files_in_out_dir) == 0
+
+        # no input videos were deleted
+        files_in_input_dir = [
+            p.relative_to(videos_path) for p in videos_path.rglob("*.*")
+        ]
+        assert len(files_in_input_dir) == files_in_input_dir_at_start
+
+    def test_redact_folder_auto_delete_input_works(
+        self, images_path: Path, redact_url, optional_api_key, tmp_path_factory
+    ):
+        # GIVEN an input dir and an output dir
+        output_path = tmp_path_factory.mktemp("vid_dir_out")
+
+        files_in_input_dir_at_start = len([p for p in images_path.rglob("*.*")])
+
+        redact_folder(
+            input_dir=images_path,
+            output_dir=output_path,
+            input_type=InputType.images,
+            output_type=OutputType.images,
+            service=ServiceType.blur,
+            redact_url=redact_url,
+            api_key=optional_api_key,
+            n_parallel_jobs=3,
+            ignore_warnings=True,
+            auto_delete_input_file=True,
+        )
+
+        # THEN no input images are anonymized in the output dir
+        files_in_out_dir = [
+            p.relative_to(output_path) for p in output_path.rglob("*.*")
+        ]
+        assert len(files_in_out_dir) == files_in_input_dir_at_start
+
+        # all input images were deleted
+        files_in_input_dir = [
+            p.relative_to(images_path) for p in images_path.rglob("*.*")
+        ]
+        assert len(files_in_input_dir) == 0