diff --git a/isic_cli/cli/image.py b/isic_cli/cli/image.py index ce3c813..0d2e141 100644 --- a/isic_cli/cli/image.py +++ b/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: diff --git a/tests/test_cli_image.py b/tests/test_cli_image.py index f1c2e60..760f0ae 100644 --- a/tests/test_cli_image.py +++ b/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"])