Merge pull request #106 from ImageMarkup/permission-errors-on-exit

Silence permission errors for deleting partially downloaded images

Author: danlamanna

isic_cli/cli/image.py

@@ -5,6 +5,7 @@
 import csv
 import functools
 import itertools
+import logging
 import os
 from pathlib import Path
 import signal
@@ -33,11 +34,29 @@
     from isic_cli.cli.context import IsicContext
 
 
+logger = logging.getLogger(__name__)
+
+
 def cleanup_partially_downloaded_files(directory: Path) -> None:
+    permission_errors = False
     for p in directory.glob(f"**/.isic-partial.{os.getpid()}.*"):
         # missing_ok=True because it's possible that another thread moved the temporary file to
         # its final destination after listing it but before unlinking.
-        p.unlink(missing_ok=True)
+        try:
+            p.unlink(missing_ok=True)
+        except PermissionError:  # noqa: PERF203
+            # frequently on windows this is raised. it appears like this could be caused by
+            # antivirus or various indexers that attempt to use the file shortly after it's
+            # created.
+            permission_errors = True
+
+    if permission_errors:
+        logger.warning(
+            click.style(
+                "Permission error while cleaning up one or more partially downloaded files",
+                fg="yellow",
+            )
+        )
 
 
 def _check_and_confirm_available_disk_space(outdir: Path, download_size: int) -> None:

tests/test_cli_image.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import logging
 import os
 from pathlib import Path
 
@@ -90,6 +91,33 @@ def test_image_download_cleanup(cli_run, outdir):
     assert not partial_file.exists()
 
 
+@pytest.mark.usefixtures("_isolated_filesystem", "_mock_images")
+def test_image_download_cleanup_permission_error(cli_run, outdir, mocker, caplog):
+    partial_file = Path(outdir) / f".isic-partial.{os.getpid()}.ISIC_0000000.jpg"
+    partial_file.parent.mkdir(parents=True)
+    partial_file.touch()
+
+    original_unlink = Path.unlink
+
+    def mock_unlink(self, *, missing_ok=False):
+        if str(partial_file) == str(self):
+            raise PermissionError("Access is denied")
+        return original_unlink(self, missing_ok=missing_ok)
+
+    mocker.patch.object(Path, "unlink", mock_unlink)
+    caplog.set_level(logging.WARNING)
+
+    result = cli_run(["image", "download", outdir])
+    assert result.exit_code == 0
+
+    # run manually since atexit won't run in the test environment
+    cleanup_partially_downloaded_files(Path(outdir))
+
+    assert (
+        "Permission error while cleaning up one or more partially downloaded files" in caplog.text
+    )
+
+
 @pytest.mark.usefixtures("_isolated_filesystem", "_mock_images")
 def test_image_download_legacy_diagnosis_unsupported(cli_run, outdir):
     result = cli_run(["image", "download", outdir, "--search", "diagnosis:melanoma"])