Merge pull request #105 from ImageMarkup/collection-type-improvements

Improve the collection parameter handling

Author: danlamanna

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"

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. "

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
 
 

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])