diff --git a/isic_cli/cli/image.py b/isic_cli/cli/image.py index 9cf4bd6..ce3c813 100644 --- a/isic_cli/cli/image.py +++ b/isic_cli/cli/image.py @@ -18,7 +18,7 @@ from rich.console import Console from rich.progress import Progress -from isic_cli.cli.types import CommaSeparatedIdentifiers, SearchString +from isic_cli.cli.types import CommaSeparatedCollectionIds, SearchString from isic_cli.cli.utils import _extract_metadata, get_attributions, suggest_guest_login from isic_cli.io.http import ( download_image, @@ -78,7 +78,7 @@ def image(ctx): @click.option( "-c", "--collections", - type=CommaSeparatedIdentifiers(), + type=CommaSeparatedCollectionIds(), default="", help=( "Filter the images based on a comma separated string of collection" diff --git a/isic_cli/cli/metadata.py b/isic_cli/cli/metadata.py index 7aff90a..156c0e1 100644 --- a/isic_cli/cli/metadata.py +++ b/isic_cli/cli/metadata.py @@ -18,7 +18,11 @@ from rich.progress import Progress, track from rich.table import Table -from isic_cli.cli.types import CommaSeparatedIdentifiers, SearchString, WritableFilePath +from isic_cli.cli.types import ( + CommaSeparatedCollectionIds, + SearchString, + WritableFilePath, +) from isic_cli.cli.utils import _extract_metadata, suggest_guest_login from isic_cli.io.http import get_images, get_num_images @@ -146,7 +150,7 @@ def validate(csv_file: io.TextIOWrapper): # noqa: C901, PLR0915, PLR0912 @click.option( "-c", "--collections", - type=CommaSeparatedIdentifiers(), + type=CommaSeparatedCollectionIds(), default="", help=( "Filter the images based on a comma separated list of collection ids e.g. 2,17,42. " diff --git a/isic_cli/cli/types.py b/isic_cli/cli/types.py index 38578f2..cc643d0 100644 --- a/isic_cli/cli/types.py +++ b/isic_cli/cli/types.py @@ -36,14 +36,34 @@ def convert(self, value, param, ctx): return value -class CommaSeparatedIdentifiers(click.ParamType): - name = "comma_separated_identifiers" +class CommaSeparatedCollectionIds(click.ParamType): + name = "comma_separated_collection_ids" def convert(self, value, param, ctx): value = super().convert(value, param, ctx) if value != "" and not re.match(r"^(\d+)(,\d+)*$", value): self.fail(f'Improperly formatted value "{value}".', param, ctx) + + collection_ids = value.split(",") if value else [] + + for collection_id in collection_ids: + try: + get_collection(ctx.obj.session, collection_id) + except HTTPError as e: # noqa: PERF203 + if e.response.status_code == 404: + append = "" + if not ctx.obj.user: + append = "Logging in may help (see `isic user login`)." + + self.fail( + f"Collection {collection_id} does not exist or you don't have access to it. {append}", # noqa: E501 + param, + ctx, + ) + else: + raise + return value diff --git a/tests/test_cli_image.py b/tests/test_cli_image.py index 889d573..f1c2e60 100644 --- a/tests/test_cli_image.py +++ b/tests/test_cli_image.py @@ -4,6 +4,7 @@ from pathlib import Path import pytest +from requests import HTTPError from isic_cli.cli.image import cleanup_partially_downloaded_files @@ -51,6 +52,19 @@ def test_image_download(cli_run, outdir): assert Path(f"{outdir}/licenses/CC-0.txt").exists() +@pytest.mark.usefixtures("_isolated_filesystem", "_mock_images") +def test_image_download_no_collection(mocker, cli_run, outdir): + mocker.patch( + "isic_cli.cli.types.get_collection", + side_effect=HTTPError(response=mocker.MagicMock(status_code=404)), + ) + + result = cli_run(["image", "download", outdir, "--collections", "462"]) + + assert result.exit_code == 2, result.exception + assert "does not exist or you don't have access to it." in result.output + + @pytest.mark.usefixtures("_isolated_filesystem", "_mock_images") def test_image_download_metadata_newlines(cli_run, outdir): result = cli_run(["image", "download", outdir])