You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/02 10:27:21 UTC

[GitHub] [iceberg] Fokko opened a new pull request, #5417: Python: Add a CLI to go through the catalog

Fokko opened a new pull request, #5417:
URL: https://github.com/apache/iceberg/pull/5417

   A CLI using Click for argument parsing, and Rich for nice formatting. When installing pyiceberg, the pyiceberg executable will be available on the PATH to go through the catalog.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r946174833


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,404 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=broad-except,redefined-builtin,redefined-outer-name
+import os
+from typing import (
+    Dict,
+    Literal,
+    Optional,
+    Tuple,
+    Type,
+)
+
+import click
+from click import Context
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchPropertyException, NoSuchTableError
+
+SUPPORTED_CATALOGS: Dict[str, Type[Catalog]] = {"thrift": HiveCatalog, "http": RestCatalog}
+
+
+@click.group()
+@click.option("--catalog", default=None)
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri")
+@click.option("--credential")
+@click.pass_context
+def run(ctx: Context, catalog: Optional[str], output: str, uri: Optional[str], credential: Optional[str]):
+    uri_env_var = "PYICEBERG_URI"
+    credential_env_var = "PYICEBERG_CREDENTIAL"
+
+    if not uri:
+        uri = os.environ.get(uri_env_var)
+    if not credential:
+        credential = os.environ.get(credential_env_var)
+
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    else:
+        ctx.obj["output"] = JsonOutput()
+
+    if not uri:
+        ctx.obj["output"].exception(
+            ValueError(f"Missing uri. Please provide using --uri or using environment variable {uri_env_var}")
+        )
+        ctx.exit(1)
+
+    assert uri  # for mypy
+
+    for scheme, catalog_type in SUPPORTED_CATALOGS.items():
+        if uri.startswith(scheme):
+            ctx.obj["catalog"] = catalog_type(catalog, uri=uri, credential=credential)  # type: ignore
+            break
+
+    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")
+        )
+        ctx.exit(1)
+
+
+def _catalog_and_output(ctx: Context) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx: Context, parent: Optional[str]):  # pylint: disable=redefined-builtin
+    """Lists tables or namespaces"""
+    catalog, output = _catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    try:
+        if parent:
+            identifiers = catalog.list_tables(parent)
+        else:
+            identifiers = catalog.list_namespaces()
+        output.identifiers(identifiers)
+    except Exception as exc:
+        output.exception(exc)
+        ctx.exit(1)
+
+
+@run.command()
+@click.option("--entity", type=click.Choice(["any", "namespace", "table"]), default="any")
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx: Context, entity: Literal["name", "namespace", "table"], identifier: str):
+    """Describes a namespace xor table"""
+    catalog, output = _catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if entity in {"namespace", "any"} and len(identifier_tuple) > 0:
+        try:
+            namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+            output.describe_properties(namespace_properties)
+            ctx.exit(0)
+        except NoSuchNamespaceError as exc:
+            if entity != "any":
+                output.exception(exc)
+                ctx.exit(1)
+
+    if entity in {"table", "any"} and len(identifier_tuple) > 1:
+        try:
+            catalog_table = catalog.load_table(identifier)
+            output.describe_table(catalog_table)
+            ctx.exit(0)
+        except NoSuchTableError as exc:
+            output.exception(exc)
+            ctx.exit(1)

Review Comment:
   `click` doesn't have any handling for setting the return value based on thrown exceptions? It would be nice to not need all of the `exit` calls.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942599139


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,159 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    @property
+    def _table(self) -> RichTable:
+        return RichTable.grid(padding=(0, 2))
+
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = self._table
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = self._table
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = self._table
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))

Review Comment:
   https://github.com/apache/iceberg/pull/5490



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941895027


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))
+
+
+@run.command()
+@click.option("--type", type=click.Choice(["any", "namespace", "table"]), default="any")
+@click.argument("identifier")
+@click.pass_context
+def properties(ctx, type: Literal["name", "namespace", "table"], identifier: str):
+    """Fetches the properties from the namespace/table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    found_namespace = False
+    try:
+        namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+        output.describe_properties(namespace_properties)
+        found_namespace = True
+    except NoSuchNamespaceError as exc:
+        if type != "any":
+            output.exception(exc)
+
+    found_table = False
+    # We expect a namespace
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier_tuple)
+            assert table.metadata
+            output.describe_properties(table.metadata.properties)
+            found_table = True
+        except NoSuchTableError as exc:
+            if type != "any":
+                output.exception(exc)
+
+    if type == "any" and not found_namespace and not found_table:
+        output.exception(NoSuchNamespaceError(f"Could not find table/namespace with identifier {identifier}"))
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def schema(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.schema(metadata.current_schema())
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def spec(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.spec(metadata.current_partition_spec())
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def uuid(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.uuid(metadata.table_uuid)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def location(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.text(metadata.location)
+
+
+@run.group()
+def drop(ctx):
+    pass
+
+
+@drop.command()
+@click.argument("table")
+@click.pass_context
+def table(ctx, table: str):
+    """Drop table"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.drop_namespace(table)
+        output.text(f"Dropped table: {table}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@drop.command()
+@click.argument("namespace")
+@click.pass_context
+def namespace(ctx, namespace: str):
+    """Drop namespace"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.drop_namespace(namespace)
+        output.text(f"Dropped namespace: {namespace}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@run.command()
+@click.argument("from_table")
+@click.argument("to_table")
+@click.pass_context
+def rename_table(ctx, from_table: str, to_table: str):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.rename_table(from_table, to_table)
+        output.text(f"Table {from_table} has been renamed to {to_table}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@run.command()
+@click.argument("from_identifier")
+@click.argument("to_identifier")
+@click.pass_context
+def table(ctx, from_identifier: str, to_identifier: str):

Review Comment:
   Do we need this and `rename_table`? I'd probably go with just `rename` for the command.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941892505


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))

Review Comment:
   This is a little confusing because someone may think they can use this command to describe a namespace. When I do that and pass a namspace name, I get this:
   
   ```
   [blue@work python]$ poetry run pyiceberg describe taxi
   Expected a namespace, got: taxi
   ```
   
   I think the message should be `f"Expected a table identifier with a namespace, but got: {identifier}"`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941012574


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)
+            for key, value in properties.items():
+                table.add_row(key, value)
+            console.print(table)
+        else:
+            console.print("No properties...")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def drop_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        catalog.drop_namespace(namespace)
+        console.print(f"Namespace {namespace} has been dropped")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")

Review Comment:
   Thanks 👍🏻 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r946664640


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,404 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=broad-except,redefined-builtin,redefined-outer-name
+import os
+from typing import (
+    Dict,
+    Literal,
+    Optional,
+    Tuple,
+    Type,
+)
+
+import click
+from click import Context
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchPropertyException, NoSuchTableError
+
+SUPPORTED_CATALOGS: Dict[str, Type[Catalog]] = {"thrift": HiveCatalog, "http": RestCatalog}
+
+
+@click.group()
+@click.option("--catalog", default=None)
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri")
+@click.option("--credential")
+@click.pass_context
+def run(ctx: Context, catalog: Optional[str], output: str, uri: Optional[str], credential: Optional[str]):
+    uri_env_var = "PYICEBERG_URI"
+    credential_env_var = "PYICEBERG_CREDENTIAL"
+
+    if not uri:
+        uri = os.environ.get(uri_env_var)
+    if not credential:
+        credential = os.environ.get(credential_env_var)
+
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    else:
+        ctx.obj["output"] = JsonOutput()
+
+    if not uri:
+        ctx.obj["output"].exception(
+            ValueError(f"Missing uri. Please provide using --uri or using environment variable {uri_env_var}")
+        )
+        ctx.exit(1)
+
+    assert uri  # for mypy
+
+    for scheme, catalog_type in SUPPORTED_CATALOGS.items():
+        if uri.startswith(scheme):
+            ctx.obj["catalog"] = catalog_type(catalog, uri=uri, credential=credential)  # type: ignore
+            break
+
+    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")
+        )
+        ctx.exit(1)
+
+
+def _catalog_and_output(ctx: Context) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx: Context, parent: Optional[str]):  # pylint: disable=redefined-builtin
+    """Lists tables or namespaces"""
+    catalog, output = _catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    try:
+        if parent:
+            identifiers = catalog.list_tables(parent)
+        else:
+            identifiers = catalog.list_namespaces()
+        output.identifiers(identifiers)
+    except Exception as exc:
+        output.exception(exc)
+        ctx.exit(1)
+
+
+@run.command()
+@click.option("--entity", type=click.Choice(["any", "namespace", "table"]), default="any")
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx: Context, entity: Literal["name", "namespace", "table"], identifier: str):
+    """Describes a namespace xor table"""
+    catalog, output = _catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if entity in {"namespace", "any"} and len(identifier_tuple) > 0:
+        try:
+            namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+            output.describe_properties(namespace_properties)
+            ctx.exit(0)
+        except NoSuchNamespaceError as exc:
+            if entity != "any":
+                output.exception(exc)
+                ctx.exit(1)
+
+    if entity in {"table", "any"} and len(identifier_tuple) > 1:
+        try:
+            catalog_table = catalog.load_table(identifier)
+            output.describe_table(catalog_table)
+            ctx.exit(0)
+        except NoSuchTableError as exc:
+            output.exception(exc)
+            ctx.exit(1)

Review Comment:
   It is a bit more complicated because we both output plain text and json. If we just throw an exception, the exit code will also be non-zero, but we want to format the exception based on the output. I've created a PR that will introduce a decorator that will catch the exception: https://github.com/apache/iceberg/pull/5546



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939726778


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)

Review Comment:
   It is really nice how this works, but I think we should also have a config file where you can define multiple catalogs. Something like we do for Spark and other engine configs. Maybe like this:
   
   ```
   ~/.iceberg.conf
   ```
   ```bash
   # prod env
   catalog.prod.uri=https://service.io/api
   catalog.prod.credential=id:secret
   # test env
   catalog.test.uri=https://service.io/api
   catalog.test.credential=id:secret
   # legacy Hive catalog
   catalog.legacy.uri=thrift://hms.service.org
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942607508


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))

Review Comment:
   I agree, but should we return the properties? https://github.com/apache/iceberg/blob/master/open-api/rest-catalog-open-api.yaml#L1809-L1830 I did that for now



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942607508


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))

Review Comment:
   I agree, but should we return the properties? https://github.com/apache/iceberg/blob/master/open-api/rest-catalog-open-api.yaml#L1809-L1830



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941012881


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)

Review Comment:
   Fair point, I've switched to grid



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941013832


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):

Review Comment:
   Thanks, but I'll combine namespaces and tables like you suggested, so this will go away :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939716330


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)
+            for key, value in properties.items():
+                table.add_row(key, value)
+            console.print(table)
+        else:
+            console.print("No properties...")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")

Review Comment:
   Looks like you'd just need to create the console with `Console(stderr=True)` ([docs](https://rich.readthedocs.io/en/stable/console.html?highlight=stderr#error-console)).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939714754


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)
+            for key, value in properties.items():
+                table.add_row(key, value)
+            console.print(table)
+        else:
+            console.print("No properties...")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")

Review Comment:
   Can you send errors to stderr?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941894746


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,159 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    @property
+    def _table(self) -> RichTable:
+        return RichTable.grid(padding=(0, 2))
+
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = self._table
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = self._table
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = self._table
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))
+        output_table.add_row("Schema", schema_tree)
+        output_table.add_row("Snapshots", snapshot_tree)
+        output_table.add_row("Properties", table_properties)
+        Console().print(output_table)
+
+    def describe_properties(self, properties: Properties):
+        output_table = self._table
+        for k, v in properties.items():
+            output_table.add_row(k, v)
+        Console().print(output_table)
+
+    def text(self, response: str):
+        Console().print(response)
+
+    def schema(self, schema: Schema):
+        schema_tree = Tree("Schema")
+        for field in schema.fields:
+            schema_tree.add(str(field))

Review Comment:
   Do we always want the tree view? I'm undecided. We could also use a table view that would work for most schemas



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941813830


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,264 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    console = Console()
+    console.print("Welcome to PyIceberg 🐧")

Review Comment:
   Probably we want to get rid of this :p



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941890556


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )

Review Comment:
   Probably want to exit here. Othewise, I get multiple errors:
   
   ```
   [blue@work python]$ poetry run pyiceberg properties taxi.nyc_taxi_yellow
   Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI
   Traceback (most recent call last):
     File "<string>", line 1, in <module>
     File "/home/blue/.cache/pypoetry/virtualenvs/pyiceberg-_q_E1N9K-py3.10/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
       return self.main(*args, **kwargs)
     File "/home/blue/.cache/pypoetry/virtualenvs/pyiceberg-_q_E1N9K-py3.10/lib/python3.10/site-packages/click/core.py", line 1055, in main
       rv = self.invoke(ctx)
     File "/home/blue/.cache/pypoetry/virtualenvs/pyiceberg-_q_E1N9K-py3.10/lib/python3.10/site-packages/click/core.py", line 1654, in invoke
       super().invoke(ctx)
     File "/home/blue/.cache/pypoetry/virtualenvs/pyiceberg-_q_E1N9K-py3.10/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
       return ctx.invoke(self.callback, **ctx.params)
     File "/home/blue/.cache/pypoetry/virtualenvs/pyiceberg-_q_E1N9K-py3.10/lib/python3.10/site-packages/click/core.py", line 760, in invoke
       return __callback(*args, **kwargs)
     File "/home/blue/.cache/pypoetry/virtualenvs/pyiceberg-_q_E1N9K-py3.10/lib/python3.10/site-packages/click/decorators.py", line 26, in new_func
       return f(get_current_context(), *args, **kwargs)
     File "/home/blue/workspace/iceberg/python/pyiceberg/cli/console.py", line 49, in run
       if uri.startswith("http"):
   AttributeError: 'NoneType' object has no attribute 'startswith'
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941814322


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,155 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = RichTable.grid()
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = RichTable.grid()
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = RichTable.grid()
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))
+        output_table.add_row("Schema", schema_tree)
+        output_table.add_row("Snapshots", snapshot_tree)
+        output_table.add_row("Properties", table_properties)
+        Console().print(output_table)
+
+    def describe_properties(self, properties: Properties):
+        output_table = RichTable.grid()

Review Comment:
   `padding=2` gives:
   ```
   ➜  python git:(fd-python-cli) ✗ pyiceberg properties fokko.table   
   Welcome to PyIceberg 🐧
   write.delete.parquet.compression-codec  zstd
                                               
                                               
   write.metadata.compression-codec        gzip
                                               
                                               
   write.summary.partition-limit           100 
                                               
                                               
   write.parquet.compression-codec         zstd
   ```
   
   I think we want to go for `padding=(0, 2)`:
   ```
   ➜  python git:(fd-python-cli) ✗ pyiceberg properties fokko.table
   Welcome to PyIceberg 🐧
   write.delete.parquet.compression-codec  zstd
   write.metadata.compression-codec        gzip
   write.summary.partition-limit           100 
   write.parquet.compression-codec         zstd
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941895483


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))
+
+
+@run.command()
+@click.option("--type", type=click.Choice(["any", "namespace", "table"]), default="any")
+@click.argument("identifier")
+@click.pass_context
+def properties(ctx, type: Literal["name", "namespace", "table"], identifier: str):
+    """Fetches the properties from the namespace/table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    found_namespace = False
+    try:
+        namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+        output.describe_properties(namespace_properties)
+        found_namespace = True
+    except NoSuchNamespaceError as exc:
+        if type != "any":
+            output.exception(exc)
+
+    found_table = False
+    # We expect a namespace
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier_tuple)
+            assert table.metadata
+            output.describe_properties(table.metadata.properties)
+            found_table = True
+        except NoSuchTableError as exc:
+            if type != "any":
+                output.exception(exc)
+
+    if type == "any" and not found_namespace and not found_table:
+        output.exception(NoSuchNamespaceError(f"Could not find table/namespace with identifier {identifier}"))
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def schema(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.schema(metadata.current_schema())
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def spec(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.spec(metadata.current_partition_spec())
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def uuid(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.uuid(metadata.table_uuid)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def location(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.text(metadata.location)
+
+
+@run.group()
+def drop(ctx):
+    pass
+
+
+@drop.command()
+@click.argument("table")
+@click.pass_context
+def table(ctx, table: str):
+    """Drop table"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.drop_namespace(table)
+        output.text(f"Dropped table: {table}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@drop.command()
+@click.argument("namespace")
+@click.pass_context
+def namespace(ctx, namespace: str):

Review Comment:
   I like the command group!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#issuecomment-1207489316

   I like this a lot, but I think we should make some adjustments so that the output is usable with other CLI tools. It feels weird to work with a CLI that pretty-prints and can't be used in combination with `awk` and `grep` easily. I made some comments on the implementation for this. I think just using `Table.grid` in most places is a good compromise between the trade-offs. And this should route error text to stderr as well.
   
   I also thought about the command structure for a while. I definitely prefer not to mirror the API directly, unless we have an API where that is the expectation (like, the `aws s3api` functionality). Most people will use this CLI to do something, like explore a catalog or find information on a table. I think the CLI should be written to make those use cases effective and easy.
   
   For example, currently there's a `load-table` command that corresponds to `load_table` in the catalog API. But loading a table is just a first step in a series of actions to do something useful. It doesn't make sense to me to load a table as a CLI command because the caller is coming with a purpose beyond that first step, like looking at the columns of a table and their documentation. It may be that the user wants to see the schema, the partitioning, table properties, or maybe a summary of everything. All of those cases load the table, but showing the same information each time is distracting.
   
   I think that the CLI should have distinct commands that are more focused on a purpose, like `schema` to show a table schema, or `properties` to show table or namespace properties. Like these, for example:
   
   ```
   pyiceberg schema db.table
   pyiceberg spec db.table
   pyiceberg order db.table
   pyiceberg uuid db.table
   pyiceberg location db.table
   pyiceberg properties db.table
   ```
   
   We probably do want a `summary` or `describe` command that shows more information.
   
   It's also strange for the to tell the API what type of object is being operated on or expected. That's another artifact of mirroring the catalog API. When I use `ls`, I don't need to tell the command that I want to list a directory or a symlinked directory. I also don't need to tell `ls` what I'm looking for, it just lists everything. I think `pyiceberg` should work the same way. Rather than having `list-tables` and `list-namespaces`, I think we should have a single `list` command that shows the output of both API calls. We can use `[bold blue]` to format namespaces and use filters, like `--tables`, to restrict the type of objects shown. Commands that I think would work with both namespaces and tables are `describe`, `properties`, `set`, and `remove`:
   
   ```
   pyiceberg describe db
   pyiceberg describe db.table
   pyiceberg properties db
   pyiceberg properties db.table
   pyiceberg set db properties a=b
   pyiceberg set db.t1 properties a=b
   pyiceberg remove db properties a b
   pyiceberg remove db.t1 properties a b
   ```
   
   The exception to that are the `create` and `drop` commands, which should probably be explicit about what you're dropping (like `rm` vs `rmdir`): `pyiceberg drop table db.table`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939720569


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)

Review Comment:
   I think we should use `Table.grid` rather than `Table` in most cases. That way, we still get nice formatting, but the output works with piping to other tools. For example:
   
   ```python
   poetry run pyiceberg ... list-namespace | grep le
   │ examples  │
   │ movielens │
   ```
   
   With `grid`:
   
   ```python
   examples 
   movielens
   ```
   
   Grid just removes the table borders, so you still get aligned text that looks nice.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939720916


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):

Review Comment:
   Minor: should be plural, `list_namespaces` to be consistent with `list_tables`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939714782


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)
+            for key, value in properties.items():
+                table.add_row(key, value)
+            console.print(table)
+        else:
+            console.print("No properties...")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def drop_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        catalog.drop_namespace(namespace)
+        console.print(f"Namespace {namespace} has been dropped")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")

Review Comment:
   Typo: should be `exist` rather than `exists`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939720800


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)
+            for key, value in properties.items():
+                table.add_row(key, value)
+            console.print(table)
+        else:
+            console.print("No properties...")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def drop_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        catalog.drop_namespace(namespace)
+        console.print(f"Namespace {namespace} has been dropped")
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")
+    except NamespaceNotEmptyError:
+        console.print(f"Namespace is not empty: {namespace}")
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def list_tables(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+    try:
+        identifiers = catalog.list_tables(namespace)
+
+        table = Table(title="Tables")
+
+        table.add_column("Table name")
+
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        console.print(table)
+    except NoSuchNamespaceError:
+        console.print(f"Namespace does not exists: {namespace}")
+
+
+@run.command()
+@click.argument("table")
+@click.pass_context
+def load_table(ctx, table: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        tbl = catalog.load_table(table)

Review Comment:
   If we move to using `grid`, it's probably okay to continue using table here, since this is a bunch of different information mixed together.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#issuecomment-1211076671

   Okay, I think I've reviewed all of the PRs. Lots of stuff going in!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941745307


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,264 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    console = Console()
+    console.print("Welcome to PyIceberg 🐧")

Review Comment:
   Cute, but should we print a welcome message every execution?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941895739


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,159 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    @property
+    def _table(self) -> RichTable:
+        return RichTable.grid(padding=(0, 2))
+
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = self._table
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = self._table
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = self._table
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))
+        output_table.add_row("Schema", schema_tree)
+        output_table.add_row("Snapshots", snapshot_tree)
+        output_table.add_row("Properties", table_properties)
+        Console().print(output_table)
+
+    def describe_properties(self, properties: Properties):
+        output_table = self._table
+        for k, v in properties.items():
+            output_table.add_row(k, v)
+        Console().print(output_table)
+
+    def text(self, response: str):
+        Console().print(response)
+
+    def schema(self, schema: Schema):
+        schema_tree = Tree("Schema")
+        for field in schema.fields:
+            schema_tree.add(str(field))
+        Console().print(schema_tree)
+
+    def spec(self, spec: PartitionSpec):
+        pass

Review Comment:
   Will need to implement



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#issuecomment-1215927326

   Looks great! I know we're still waiting on config, but this is usable right now so I merged it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941819626


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,264 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    console = Console()
+    console.print("Welcome to PyIceberg 🐧")
+    ctx.ensure_object(dict)
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        table = catalog.load_table(identifier)
+        output.describe_table(table)
+    except NoSuchNamespaceError as exc:
+        output.exception(exc)
+
+
+@run.command()
+@click.option("--type", type=click.Choice(["all", "namespace", "table"]), default="all")

Review Comment:
   Good point, I've changed it to `--all`. We can also change it to `--table` and `--namespace`, but I like how this shows up as an option:
   ```
   ➜  python git:(fd-python-cli) ✗ pyiceberg properties --help
   Usage: pyiceberg properties [OPTIONS] IDENTIFIER
   
     Fetches the properties from the namespace/table
   
   Options:
     --type [any|namespace|table]
     --help                        Show this message and exit.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941821387


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)

Review Comment:
   I've converted this one into a draft because I'm working on a config PR in parallel.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942377596


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )

Review Comment:
   Good call, looking into the exit codes was still on my list. I also want to return a non-zero exit code when an exception is raised.
   ```
   ➜  python git:(fd-python-cli) ✗ pyiceberg properties fokko
   Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI
   ➜  python git:(fd-python-cli) ✗ echo $?
   1
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941894108


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))
+
+
+@run.command()
+@click.option("--type", type=click.Choice(["any", "namespace", "table"]), default="any")
+@click.argument("identifier")
+@click.pass_context
+def properties(ctx, type: Literal["name", "namespace", "table"], identifier: str):
+    """Fetches the properties from the namespace/table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    found_namespace = False
+    try:
+        namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+        output.describe_properties(namespace_properties)
+        found_namespace = True
+    except NoSuchNamespaceError as exc:
+        if type != "any":
+            output.exception(exc)
+
+    found_table = False
+    # We expect a namespace
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier_tuple)
+            assert table.metadata
+            output.describe_properties(table.metadata.properties)
+            found_table = True
+        except NoSuchTableError as exc:
+            if type != "any":
+                output.exception(exc)
+
+    if type == "any" and not found_namespace and not found_table:
+        output.exception(NoSuchNamespaceError(f"Could not find table/namespace with identifier {identifier}"))
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def schema(ctx, identifier: str):

Review Comment:
   I accidentally hit the single part table name here as well. We may want to add `with load(identifier) as table:` to encapsulate the `len(identifier_tuple) > 1` check and other handling above.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r946401206


##########
python/pyiceberg/catalog/hive.py:
##########
@@ -235,8 +235,8 @@ def identifier_to_database_and_table(
 
         return tuple_identifier[0], tuple_identifier[1]
 
-    def __init__(self, name: str, properties: Properties, uri: str):
-        super().__init__(name, properties)
+    def __init__(self, name: str, uri: str, **properties: str):
+        super().__init__(name, **properties)

Review Comment:
   I'm not sure if I fully understand. Right now you can pass the `uri` in as a positional argument `RestCatalog(name, uri)` and also a keyword argument `RestCatalog(name, uri=uri)`. This will automatically be mapped to `uri`. It will also raise an error when it isn't passed in. We can also go full keyword arguments (which is what a lot of Pythonistas recommend because it is more readable), but I think it is nice to define the uri explicitly because it provides autocompletion in the IDE.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#issuecomment-1216220100

   @rdblue Fine by me, thanks for merging 👍🏻 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#issuecomment-1214223712

   Added a gazillion tests, hopefully, this will bump it up to 90%+.
   
   For the json API, we could also return only pydantic models, and we get an open-api for free from that :) Might be a bit too much for this stage, but I would like to share the idea to see what others think.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939720569


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)

Review Comment:
   I think we should use `Table.grid` rather than `Table` in most cases. That way, we still get nice formatting, but the output works with piping to other tools. For example:
   
   ```python
   poetry run pyiceberg ... list-namespace | grep le
   │ examples  │
   │ movielens │
   ```
   
   With `grid`:
   
   ```python
   examples 
   movielens
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r940608374


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))

Review Comment:
   Updated (also updated this in the REST catalog).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r940600833


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")

Review Comment:
   Good point!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941772129


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,155 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = RichTable.grid()
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = RichTable.grid()
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = RichTable.grid()
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))
+        output_table.add_row("Schema", schema_tree)
+        output_table.add_row("Snapshots", snapshot_tree)
+        output_table.add_row("Properties", table_properties)
+        Console().print(output_table)
+
+    def describe_properties(self, properties: Properties):
+        output_table = RichTable.grid()

Review Comment:
   I think you need to add `padding=2` to this, or else you get columns that run together:
   
   ```
   Welcome to PyIceberg 🐧
   write.delete.parquet.compression-codecgzip
   write.metadata.compression-codec      gzip
   ...
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941891607


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,159 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    @property
+    def _table(self) -> RichTable:
+        return RichTable.grid(padding=(0, 2))
+
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = self._table
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = self._table
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = self._table
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))

Review Comment:
   As a follow-up, we should implement `__str__` on sort order so that we get a reasonable string. Right now, I'm seeing this:
   
   ```
   Sort order            order_id=1 fields=[SortField(source_id=2, transform=UnknownTransform(transform='month'), direction=<SortDirection.ASC: 'asc'>, null_order=<NullOrder.NULLS_FIRST: 'nulls-first'>), SortField(source_id=9, transform=IdentityTransform(), direction=<SortDirection.ASC: 'asc'>, null_order=<NullOrder.NULLS_FIRST: 'nulls-first'>), SortField(source_id=2,                
                         transform=IdentityTransform(), direction=<SortDirection.ASC: 'asc'>, null_order=<NullOrder.NULLS_FIRST: 'nulls-first'>), SortField(source_id=3, transform=IdentityTransform(), direction=<SortDirection.ASC: 'asc'>, null_order=<NullOrder.NULLS_FIRST: 'nulls-first'>)]
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942617547


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,159 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: UUID):
+        ...
+
+
+class ConsoleOutput(Output):
+    @property
+    def _table(self) -> RichTable:
+        return RichTable.grid(padding=(0, 2))
+
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = self._table
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = self._table
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = self._table
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))
+        output_table.add_row("Schema", schema_tree)
+        output_table.add_row("Snapshots", snapshot_tree)
+        output_table.add_row("Properties", table_properties)
+        Console().print(output_table)
+
+    def describe_properties(self, properties: Properties):
+        output_table = self._table
+        for k, v in properties.items():
+            output_table.add_row(k, v)
+        Console().print(output_table)
+
+    def text(self, response: str):
+        Console().print(response)
+
+    def schema(self, schema: Schema):
+        schema_tree = Tree("Schema")
+        for field in schema.fields:
+            schema_tree.add(str(field))

Review Comment:
   We don't really construct a tree, so a table would work as well. Maybe with the minimal table that also makes it a bit cleaner



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue merged pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5417:
URL: https://github.com/apache/iceberg/pull/5417


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r941770917


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,264 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    console = Console()
+    console.print("Welcome to PyIceberg 🐧")
+    ctx.ensure_object(dict)
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        table = catalog.load_table(identifier)
+        output.describe_table(table)
+    except NoSuchNamespaceError as exc:
+        output.exception(exc)
+
+
+@run.command()
+@click.option("--type", type=click.Choice(["all", "namespace", "table"]), default="all")

Review Comment:
   Should it be `any` rather than `all`? There should be only one resource so we won't ever have 2 things to show.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939714602


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")

Review Comment:
   I'd probably reserve `--catalog` to select a named catalog from a config file. I don't think that we need it here because we can look at the scheme of the `--uri` option to see whether we should use REST (`https`) or Hive (`thrift`).



##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))

Review Comment:
   This should be `credential` (singular)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r939720569


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,185 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+
+import click
+from rich.console import Console
+from rich.table import Table
+from rich.tree import Tree
+
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.exceptions import NamespaceNotEmptyError, NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", type=click.Choice(["rest"], case_sensitive=False), default="rest")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credentials", default=lambda: os.environ.get("PYICEBERG_CREDENTIALS"))
+@click.pass_context
+def run(ctx, catalog, uri, credentials):
+    if catalog == "rest":
+        ctx.ensure_object(dict)
+        ctx.obj["catalog"] = RestCatalog(name="catalog", properties={}, uri=uri, credentials=credentials)
+
+
+@run.command()
+@click.pass_context
+def list_namespace(ctx):
+    # Add parent namespaces
+    catalog: RestCatalog = ctx.obj["catalog"]
+    identifiers = catalog.list_namespaces()
+
+    table = Table(title="Namespaces")
+
+    table.add_column("Namespace")
+
+    for identifier in identifiers:
+        table.add_row(".".join(identifier))
+
+    console = Console()
+    console.print(table)
+
+
+@run.command()
+@click.argument("namespace")
+@click.pass_context
+def load_namespace(ctx, namespace: str):
+    catalog: RestCatalog = ctx.obj["catalog"]
+    console = Console()
+
+    try:
+        properties = catalog.load_namespace_properties(namespace)
+
+        if properties:
+            table = Table(title=f"{namespace} properties", show_header=False)

Review Comment:
   I think we should use [`Table.grid`](https://rich.readthedocs.io/en/stable/tables.html#grids) rather than `Table` in most cases. That way, we still get nice formatting, but the output works with piping to other tools. For example:
   
   ```python
   poetry run pyiceberg ... list-namespace | grep le
   │ examples  │
   │ movielens │
   ```
   
   With `grid`:
   
   ```python
   examples 
   movielens
   ```
   
   Grid just removes the table borders, so you still get aligned text that looks nice.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942617796


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))
+
+
+@run.command()
+@click.option("--type", type=click.Choice(["any", "namespace", "table"]), default="any")
+@click.argument("identifier")
+@click.pass_context
+def properties(ctx, type: Literal["name", "namespace", "table"], identifier: str):
+    """Fetches the properties from the namespace/table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    found_namespace = False
+    try:
+        namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+        output.describe_properties(namespace_properties)
+        found_namespace = True
+    except NoSuchNamespaceError as exc:
+        if type != "any":
+            output.exception(exc)
+
+    found_table = False
+    # We expect a namespace
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier_tuple)
+            assert table.metadata
+            output.describe_properties(table.metadata.properties)
+            found_table = True
+        except NoSuchTableError as exc:
+            if type != "any":
+                output.exception(exc)
+
+    if type == "any" and not found_namespace and not found_table:
+        output.exception(NoSuchNamespaceError(f"Could not find table/namespace with identifier {identifier}"))
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def schema(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.schema(metadata.current_schema())
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def spec(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.spec(metadata.current_partition_spec())
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def uuid(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.uuid(metadata.table_uuid)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def location(ctx, identifier: str):
+    catalog, output = _get_catalog_and_output(ctx)
+    metadata = catalog.load_table(identifier).metadata
+    output.text(metadata.location)
+
+
+@run.group()
+def drop(ctx):
+    pass
+
+
+@drop.command()
+@click.argument("table")
+@click.pass_context
+def table(ctx, table: str):
+    """Drop table"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.drop_namespace(table)
+        output.text(f"Dropped table: {table}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@drop.command()
+@click.argument("namespace")
+@click.pass_context
+def namespace(ctx, namespace: str):
+    """Drop namespace"""
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.drop_namespace(namespace)
+        output.text(f"Dropped namespace: {namespace}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@run.command()
+@click.argument("from_table")
+@click.argument("to_table")
+@click.pass_context
+def rename_table(ctx, from_table: str, to_table: str):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    try:
+        catalog.rename_table(from_table, to_table)
+        output.text(f"Table {from_table} has been renamed to {to_table}")
+    except Exception as exc:
+        output.exception(exc)
+
+
+@run.command()
+@click.argument("from_identifier")
+@click.argument("to_identifier")
+@click.pass_context
+def table(ctx, from_identifier: str, to_identifier: str):

Review Comment:
   Yes, this was also caught by the linter 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] Fokko commented on pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#issuecomment-1210903566

   I think we're almost there. I marked it as a draft because it is still a bit dirty. For example, setting properties is still missing, and it had some rough edges. Would be great if we can merge in some of the other PRs that also allow us to clean this one up:
   
   - [Python: Add __str__ implementation to SortOrder and SortField](https://github.com/apache/iceberg/pull/5490)
   - [Python: Add configuration](https://github.com/apache/iceberg/pull/5488)
   - [Python: Checks on REST catalog table/namespace identifiers](https://github.com/apache/iceberg/pull/5485)
   - [Python: Add support for hierarchical namespaces](https://github.com/apache/iceberg/pull/5467)
   - [Python:  Implement Hive create and load table](https://github.com/apache/iceberg/pull/5447) (so we can make the `metadata` property required, and we don't have to do the assertions everywhere 👍🏻 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r942752247


##########
python/pyiceberg/cli/console.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from typing import Literal, Optional, Tuple
+
+import click
+
+from pyiceberg.catalog import Catalog
+from pyiceberg.catalog.hive import HiveCatalog
+from pyiceberg.catalog.rest import RestCatalog
+from pyiceberg.cli.output import ConsoleOutput, JsonOutput, Output
+from pyiceberg.exceptions import NoSuchNamespaceError, NoSuchTableError
+
+
+@click.group()
+@click.option("--catalog", default="catalog")
+@click.option("--output", type=click.Choice(["text", "json"]), default="text")
+@click.option("--uri", default=lambda: os.environ.get("PYICEBERG_URI"))
+@click.option("--credential", default=lambda: os.environ.get("PYICEBERG_CREDENTIAL"))
+@click.pass_context
+def run(ctx, catalog: str, output: str, uri: str, credential: str):
+    ctx.ensure_object(dict)
+    if output == "text":
+        ctx.obj["output"] = ConsoleOutput()
+    elif output == "json":
+        ctx.obj["output"] = JsonOutput()
+    else:
+        raise ValueError(f"Unknown output: {output}")
+
+    if not uri:
+        ctx.obj["output"].exception(
+            f"Missing uri, cannot connect to catalog. Please provide using --uri or by setting the environment variable PYICEBERG_URI"
+        )
+
+    if uri.startswith("http"):
+        ctx.obj["catalog"] = RestCatalog(name=catalog, properties={}, uri=uri, credential=credential)
+    elif uri.startswith("thrift"):
+        ctx.obj["catalog"] = HiveCatalog(name=catalog, properties={}, uri=uri)
+    else:
+        raise ValueError("Could not determine catalog type from uri. REST (http/https) and Hive (thrift) is supported")
+
+
+def _get_catalog_and_output(ctx) -> Tuple[Catalog, Output]:
+    """
+    Small helper to set the types
+    """
+    return ctx.obj["catalog"], ctx.obj["output"]
+
+
+@run.command()
+@click.pass_context
+@click.argument("parent", required=False)
+def list(ctx, parent: Optional[str]):
+    catalog, output = _get_catalog_and_output(ctx)
+
+    # still wip, will become more beautiful
+    # https://github.com/apache/iceberg/pull/5467/
+    # has been merged
+    if parent:
+        identifiers = catalog.list_tables(parent)
+    else:
+        identifiers = catalog.list_namespaces()
+
+    output.identifiers(identifiers)
+
+
+@run.command()
+@click.argument("identifier")
+@click.pass_context
+def describe(ctx, identifier: str):
+    """Describes a table"""
+    catalog, output = _get_catalog_and_output(ctx)
+    identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+    if len(identifier_tuple) > 1:
+        try:
+            table = catalog.load_table(identifier)
+            output.describe_table(table)
+        except NoSuchNamespaceError as exc:
+            output.exception(exc)
+    else:
+        output.exception(NoSuchTableError(f"Expected a namespace, got: {identifier}"))

Review Comment:
   Yes, I like being able to use `describe` on any object much better!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r946175462


##########
python/pyiceberg/cli/output.py:
##########
@@ -0,0 +1,163 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+from abc import ABC, abstractmethod
+from typing import Any, List, Optional
+from uuid import UUID
+
+from rich.console import Console
+from rich.table import Table as RichTable
+from rich.tree import Tree
+
+from pyiceberg.schema import Schema
+from pyiceberg.table import Table
+from pyiceberg.table.partitioning import PartitionSpec
+from pyiceberg.typedef import Identifier, Properties
+
+
+class Output(ABC):
+    """Output interface for exporting"""
+
+    @abstractmethod
+    def exception(self, ex: Exception):
+        ...
+
+    @abstractmethod
+    def identifiers(self, identifiers: List[Identifier]):
+        ...
+
+    @abstractmethod
+    def describe_table(self, table):
+        ...
+
+    @abstractmethod
+    def describe_properties(self, properties: Properties):
+        ...
+
+    @abstractmethod
+    def text(self, response: str):
+        ...
+
+    @abstractmethod
+    def schema(self, schema: Schema):
+        ...
+
+    @abstractmethod
+    def spec(self, spec: PartitionSpec):
+        ...
+
+    @abstractmethod
+    def uuid(self, uuid: Optional[UUID]):
+        ...
+
+
+class ConsoleOutput(Output):
+    """Writes to the console"""
+
+    @property
+    def _table(self) -> RichTable:
+        return RichTable.grid(padding=(0, 2))
+
+    def exception(self, ex: Exception):
+        Console(stderr=True).print(ex)
+
+    def identifiers(self, identifiers: List[Identifier]):
+        table = self._table
+        for identifier in identifiers:
+            table.add_row(".".join(identifier))
+
+        Console().print(table)
+
+    def describe_table(self, table: Table):
+        assert table.metadata
+        metadata = table.metadata
+        table_properties = self._table
+
+        for key, value in metadata.properties.items():
+            table_properties.add_row(key, value)
+
+        schema_tree = Tree("Schema")
+        for field in metadata.current_schema().fields:
+            schema_tree.add(str(field))
+
+        snapshot_tree = Tree("Snapshots")
+        for snapshot in metadata.snapshots:
+            snapshot_tree.add(f"Snapshot {snapshot.schema_id}: {snapshot.manifest_list}")
+
+        output_table = self._table
+        output_table.add_row("Table format version", str(metadata.format_version))
+        output_table.add_row("Metadata location", table.metadata_location)
+        output_table.add_row("Table UUID", str(table.metadata.table_uuid))
+        output_table.add_row("Last Updated", str(metadata.last_updated_ms))
+        output_table.add_row("Partition spec", str(metadata.current_partition_spec()))
+        output_table.add_row("Sort order", str(metadata.current_sort_order()))
+        output_table.add_row("Schema", schema_tree)
+        output_table.add_row("Snapshots", snapshot_tree)
+        output_table.add_row("Properties", table_properties)
+        Console().print(output_table)
+
+    def describe_properties(self, properties: Properties):
+        output_table = self._table
+        for k, v in properties.items():
+            output_table.add_row(k, v)
+        Console().print(output_table)
+
+    def text(self, response: str):
+        Console().print(response)
+
+    def schema(self, schema: Schema):
+        output_table = self._table
+        for field in schema.fields:
+            output_table.add_row(field.name, str(field.field_type))

Review Comment:
   We should also include the field docs as a column.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rdblue commented on a diff in pull request #5417: Python: Add a CLI to go through the catalog

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5417:
URL: https://github.com/apache/iceberg/pull/5417#discussion_r946173750


##########
python/pyiceberg/catalog/hive.py:
##########
@@ -235,8 +235,8 @@ def identifier_to_database_and_table(
 
         return tuple_identifier[0], tuple_identifier[1]
 
-    def __init__(self, name: str, properties: Properties, uri: str):
-        super().__init__(name, properties)
+    def __init__(self, name: str, uri: str, **properties: str):
+        super().__init__(name, **properties)

Review Comment:
   We will probably want to move everything into properties, like Pydantic does. I don't think it makes much sense to make the caller pull out certain properties, like `uri`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org