Fix CLI command order and catalog not found error

pyiceberg/catalog/__init__.py

@@ -43,6 +43,7 @@
     NoSuchTableError,
     NotInstalledError,
     TableAlreadyExistsError,
+    NoCatalogError
 )
 from pyiceberg.io import FileIO, load_file_io
 from pyiceberg.manifest import ManifestFile
@@ -234,9 +235,14 @@ 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)
-
+    
     if catalog_impl := properties.get(PY_CATALOG_IMPL):
         if provided_catalog_type:
             raise ValueError(
@@ -255,7 +261,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 NoSuchNamespaceError, NoSuchPropertyException, NoSuchTableError, NoCatalogError
 from pyiceberg.table import TableProperties
 from pyiceberg.table.refs import SnapshotRef
 from pyiceberg.utils.properties import property_as_int
@@ -43,6 +43,11 @@ def decorator(func: Callable) -> Callable:  # type: ignore
         def wrapper(*args: Any, **kwargs: Any):  # type: ignore
             try:
                 return func(*args, **kwargs)
+            except NoCatalogError as e:
+                ctx: Context = click.get_current_context(silent=True)
+                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)
@@ -86,31 +91,48 @@ 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:
+    if ctx.obj["first_level_catalog"] is None and ctx.obj["second_level_catalog"] is None:
+        ctx.obj["catalog"] = load_catalog(None, **ctx.obj["properties"])
+    elif ctx.obj["first_level_catalog"] is not None and ctx.obj["second_level_catalog"] is None:
+        ctx.obj["catalog"] = load_catalog(ctx.obj["first_level_catalog"],**ctx.obj["properties"])
+    else:
+        ctx.obj["catalog"] = load_catalog(ctx.obj["second_level_catalog"],**ctx.obj["properties"])
+        print("cataloglll",ctx.obj["catalog"]) 
+
+
     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")
+        ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
         )
         ctx.exit(1)
 
-
 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()
+@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(ctx: Context, catalog: Optional[str], parent: Optional[str]) -> None:  # pylint: disable=redefined-builtin
     """List tables or namespaces."""
+    if ctx.obj["first_level_catalog"] == None:
+        ctx.obj["second_level_catalog"] = catalog
+
     catalog, output = _catalog_and_output(ctx)
-
     identifiers = []
     if parent:
         # Do we have tables under parent namespace?

pyiceberg/exceptions.py

@@ -27,6 +27,8 @@ class NamespaceNotEmptyError(Exception):
 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

@@ -35,18 +35,28 @@
 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)
 def env_vars(mocker: MockFixture) -> None:
@@ -115,6 +125,20 @@ def test_list_namespace(catalog: InMemoryCatalog) -> None:
     assert result.exit_code == 0
     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)