Fix CLI command order and catalog not found error

pyiceberg/catalog/__init__.py

@@ -39,6 +39,7 @@
 
 from pyiceberg.exceptions import (
     NamespaceAlreadyExistsError,
+    NoCatalogError,
     NoSuchNamespaceError,
     NoSuchTableError,
     NotInstalledError,
@@ -234,6 +235,9 @@ def load_catalog(name: Optional[str] = None, **properties: Optional[str]) -> Cat
     env = _ENV_CONFIG.get_catalog_config(name)
     conf: RecursiveDict = merge_config(env or {}, cast(RecursiveDict, properties))
 
+    if conf == {}:
+        raise NoCatalogError(f"{name} catalog not found, please provide a catalog type using --type")
+
     catalog_type: Optional[CatalogType]
     provided_catalog_type = conf.get(TYPE)
 
@@ -255,7 +259,6 @@ def load_catalog(name: Optional[str] = None, **properties: Optional[str]) -> Cat
         catalog_type = CatalogType(provided_catalog_type.lower())
     elif not provided_catalog_type:
         catalog_type = infer_catalog_type(name, conf)
-
     if catalog_type:
         return AVAILABLE_CATALOGS[catalog_type](name, cast(Dict[str, str], conf))
 

pyiceberg/cli/console.py

@@ -31,7 +31,7 @@
 from pyiceberg import __version__
 from pyiceberg.catalog import Catalog, load_catalog
 from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
-from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchPropertyException, NoSuchTableError
+from pyiceberg.exceptions import NoCatalogError, NoSuchNamespaceError, NoSuchPropertyException, NoSuchTableError
 from pyiceberg.table import TableProperties
 from pyiceberg.table.refs import SnapshotRef
 from pyiceberg.utils.properties import property_as_int
@@ -42,9 +42,13 @@ def decorator(func: Callable) -> Callable:  # type: ignore
         @wraps(func)
         def wrapper(*args: Any, **kwargs: Any):  # type: ignore
             try:
+                ctx: Context = click.get_current_context(silent=True)
                 return func(*args, **kwargs)
+            except NoCatalogError as e:
+                ctx.obj["output"].exception(e)
+                ctx.exit(1)
+
             except Exception as e:
-                ctx: Context = click.get_current_context(silent=True)
                 _, output = _catalog_and_output(ctx)
                 output.exception(e)
                 ctx.exit(1)
@@ -86,11 +90,19 @@ def run(
         ctx.obj["output"] = JsonOutput(verbose=verbose)
 
     try:
-        ctx.obj["catalog"] = load_catalog(catalog, **properties)
+        ctx.obj["first_level_catalog"] = catalog
+        ctx.obj["second_level_catalog"] = None
+        ctx.obj["properties"] = properties
+
     except Exception as e:
         ctx.obj["output"].exception(e)
         ctx.exit(1)
 
+
+def _load_the_catalog_with_first_level_catalog(ctx: Context) -> None:
+    catalog_option = ctx.obj.get("first_level_catalog", None) or ctx.obj.get("second_level_catalog", None)
+    ctx.obj["catalog"] = load_catalog(catalog_option, **ctx.obj["properties"])
+
     if not isinstance(ctx.obj["catalog"], Catalog):
         ctx.obj["output"].exception(
             ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
@@ -99,25 +111,32 @@ def run(
 
 
 def _catalog_and_output(ctx: Context) -> Tuple[Catalog, Output]:
-    """Small helper to set the types."""
+    """Small helper to set the types and load the catalog."""
+    _load_the_catalog_with_first_level_catalog(ctx)
     return ctx.obj["catalog"], ctx.obj["output"]
 
 
-@run.command()
+@run.command(name="list")
+@click.option("--catalog")
 @click.pass_context
 @click.argument("parent", required=False)
 @catch_exception()
-def list(ctx: Context, parent: Optional[str]) -> None:  # pylint: disable=redefined-builtin
+def list_entities(ctx: Context, catalog: Optional[str], parent: Optional[str]) -> None:  # pylint: disable=redefined-builtin
     """List tables or namespaces."""
-    catalog, output = _catalog_and_output(ctx)
+    if ctx.obj["first_level_catalog"] is None:
+        ctx.obj["second_level_catalog"] = catalog
+
+    catalog_obj, output = _catalog_and_output(ctx)
 
     identifiers = []
+
     if parent:
         # Do we have tables under parent namespace?
-        identifiers = catalog.list_tables(parent)
+        identifiers = catalog_obj.list_tables(parent)
+
     if not identifiers:
         # List hierarchical namespaces if parent, root namespaces otherwise.
-        identifiers = catalog.list_namespaces(parent or ())
+        identifiers = catalog_obj.list_namespaces(parent or ())
     output.identifiers(identifiers)
 
 

pyiceberg/exceptions.py

@@ -28,6 +28,10 @@ class NamespaceAlreadyExistsError(Exception):
     """Raised when a name-space being created already exists in the catalog."""
 
 
+class NoCatalogError(Exception):
+    """Raised when no catalog is set."""
+
+
 class ValidationError(Exception):
     """Raises when there is an issue with the schema."""
 

tests/cli/test_console.py

@@ -33,19 +33,34 @@
 from pyiceberg.transforms import IdentityTransform
 from pyiceberg.typedef import Properties
 from pyiceberg.types import LongType, NestedField
-from pyiceberg.utils.config import Config
+
+
+def test_no_catalog_error(mocker: MockFixture, empty_home_dir_path: str) -> None:
+    mocker.patch.dict(os.environ, values={"HOME": empty_home_dir_path, "PYICEBERG_HOME": empty_home_dir_path})
+    runner = CliRunner()
+    result = runner.invoke(run, ["list", "--catalog", "nocatalog"], catch_exceptions=False)
+    assert result.exit_code == 1
+    assert result.output == "nocatalog catalog not found, please provide a catalog type using --type\n"
 
 
 def test_missing_uri(mocker: MockFixture, empty_home_dir_path: str) -> None:
     # mock to prevent parsing ~/.pyiceberg.yaml or {PYICEBERG_HOME}/.pyiceberg.yaml
     mocker.patch.dict(os.environ, values={"HOME": empty_home_dir_path, "PYICEBERG_HOME": empty_home_dir_path})
-    mocker.patch("pyiceberg.catalog._ENV_CONFIG", return_value=Config())
+    mocker.patch(
+        "pyiceberg.catalog._ENV_CONFIG.get_catalog_config",
+        return_value={
+            "catalog": {"production": {}},
+            "home": empty_home_dir_path,
+        },
+    )
 
     runner = CliRunner()
-    result = runner.invoke(run, ["list"])
-
-    assert result.exit_code == 1
-    assert result.output == "Could not initialize catalog with the following properties: {}\n"
+    with pytest.raises(
+        ValueError,
+        match="URI missing, please provide using --uri, the config or environment variable PYICEBERG_CATALOG__PRODUCTION__URI",
+    ):
+        result = runner.invoke(run, ["list", "--catalog", "production"], catch_exceptions=False)
+        assert result.exit_code == 1
 
 
 @pytest.fixture(autouse=True)
@@ -116,6 +131,21 @@ def test_list_namespace(catalog: InMemoryCatalog) -> None:
     assert result.output == "default.my_table\n"
 
 
+def test_list_with_catalog_option_wrong_order(catalog: InMemoryCatalog):
+    """Test that `pyiceberg list --catalog hive` raises an error due to wrong order."""
+    catalog.create_namespace(TEST_TABLE_NAMESPACE)
+    catalog.create_table(
+        identifier=TEST_TABLE_IDENTIFIER,
+        schema=TEST_TABLE_SCHEMA,
+        partition_spec=TEST_TABLE_PARTITION_SPEC,
+        properties=TEST_TABLE_PROPERTIES,
+    )
+    runner = CliRunner()
+    result = runner.invoke(run, ["list", "--catalog", "hive"])
+    assert result.exit_code == 0
+    print("result.output", result.output)
+
+
 def test_describe_namespace(catalog: InMemoryCatalog, namespace_properties: Properties) -> None:
     catalog.create_namespace(TEST_TABLE_NAMESPACE, namespace_properties)