You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by fo...@apache.org on 2022/09/14 15:08:43 UTC
[iceberg] branch master updated: Python: Make Get Properties CLI options consistent. (#5736)
This is an automated email from the ASF dual-hosted git repository.
fokko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new 0f0b1af2eb Python: Make Get Properties CLI options consistent. (#5736)
0f0b1af2eb is described below
commit 0f0b1af2ebbc5693ff6dc8049a1e3490540311f8
Author: Dhruv Pratap <dp...@netflix.com>
AuthorDate: Wed Sep 14 11:08:37 2022 -0400
Python: Make Get Properties CLI options consistent. (#5736)
Consistent with Set and Remove CLI options
* Python: Make Get Properties CLI options consistent with Set and Remove CLI Options
* Python: Explicitly specify command names in the annotations and not infer it from the method name to fix F811 lint issues of duplicate method declaration.
---
python/pyiceberg/cli/console.py | 74 +++++++++++++++++++---------------------
python/tests/cli/test_console.py | 46 +++++++++++++++++--------
2 files changed, 68 insertions(+), 52 deletions(-)
diff --git a/python/pyiceberg/cli/console.py b/python/pyiceberg/cli/console.py
index 404a882d9e..3a040159d8 100644
--- a/python/pyiceberg/cli/console.py
+++ b/python/pyiceberg/cli/console.py
@@ -227,55 +227,53 @@ def properties():
"""Properties on tables/namespaces"""
-@properties.command()
-@click.option("--entity", type=click.Choice(["any", "namespace", "table"]), default="any")
+@properties.group()
+def get():
+ """Fetch properties on tables/namespaces"""
+
+
+@get.command("namespace")
@click.argument("identifier")
@click.argument("property_name", required=False)
@click.pass_context
@catch_exception()
-def get(ctx: Context, entity: Literal["name", "namespace", "table"], identifier: str, property_name: str):
- """Fetches a property of a namespace or table"""
+def get_namespace(ctx: Context, identifier: str, property_name: str):
+ """Fetch properties on a namespace"""
catalog, output = _catalog_and_output(ctx)
identifier_tuple = Catalog.identifier_to_tuple(identifier)
- is_namespace = False
- if entity in {"namespace", "any"}:
- try:
- namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+ namespace_properties = catalog.load_namespace_properties(identifier_tuple)
+ assert namespace_properties
- if property_name:
- if property_value := namespace_properties.get(property_name):
- output.text(property_value)
- is_namespace = True
- else:
- raise NoSuchPropertyException(f"Could not find property {property_name} on namespace {identifier}")
- else:
- output.describe_properties(namespace_properties)
- is_namespace = True
- except NoSuchNamespaceError as exc:
- if entity != "any" or len(identifier_tuple) <= 1: # type: ignore
- raise exc
- is_table = False
- if is_namespace is False and len(identifier_tuple) > 1 and entity in {"table", "any"}:
- metadata = catalog.load_table(identifier_tuple).metadata
- assert metadata
-
- if property_name:
- if property_value := metadata.properties.get(property_name):
- output.text(property_value)
- is_table = True
- else:
- raise NoSuchPropertyException(f"Could not find property {property_name} on table {identifier}")
+ if property_name:
+ if property_value := namespace_properties.get(property_name):
+ output.text(property_value)
else:
- output.describe_properties(metadata.properties)
- is_table = True
+ raise NoSuchPropertyException(f"Could not find property {property_name} on namespace {identifier}")
+ else:
+ output.describe_properties(namespace_properties)
- if is_namespace is False and is_table is False:
- property_err = ""
- if property_name:
- property_err = f" with property {property_name}"
- raise NoSuchNamespaceError(f"Table or namespace does not exist: {identifier}{property_err}")
+@get.command("table")
+@click.argument("identifier")
+@click.argument("property_name", required=False)
+@click.pass_context
+@catch_exception()
+def get_table(ctx: Context, identifier: str, property_name: str):
+ """Fetch properties on a table"""
+ catalog, output = _catalog_and_output(ctx)
+ identifier_tuple = Catalog.identifier_to_tuple(identifier)
+
+ metadata = catalog.load_table(identifier_tuple).metadata
+ assert metadata
+
+ if property_name:
+ if property_value := metadata.properties.get(property_name):
+ output.text(property_value)
+ else:
+ raise NoSuchPropertyException(f"Could not find property {property_name} on table {identifier}")
+ else:
+ output.describe_properties(metadata.properties)
@properties.group()
diff --git a/python/tests/cli/test_console.py b/python/tests/cli/test_console.py
index 7c57e7b26e..38382cd1bb 100644
--- a/python/tests/cli/test_console.py
+++ b/python/tests/cli/test_console.py
@@ -357,7 +357,7 @@ def test_rename_table_does_not_exists(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_get_table(_):
runner = CliRunner()
- result = runner.invoke(run, ["properties", "get", "default.foo"])
+ result = runner.invoke(run, ["properties", "get", "table", "default.foo"])
assert result.exit_code == 0
assert result.output == "read.split.target.size 134217728\n"
@@ -366,7 +366,7 @@ def test_properties_get_table(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_get_table_specific_property(_):
runner = CliRunner()
- result = runner.invoke(run, ["properties", "get", "default.foo", "read.split.target.size"])
+ result = runner.invoke(run, ["properties", "get", "table", "default.foo", "read.split.target.size"])
assert result.exit_code == 0
assert result.output == "134217728\n"
@@ -375,7 +375,7 @@ def test_properties_get_table_specific_property(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_get_table_specific_property_that_doesnt_exist(_):
runner = CliRunner()
- result = runner.invoke(run, ["properties", "get", "default.foo", "doesnotexist"])
+ result = runner.invoke(run, ["properties", "get", "table", "default.foo", "doesnotexist"])
assert result.exit_code == 1
assert result.output == "Could not find property doesnotexist on table default.foo\n"
@@ -384,16 +384,16 @@ def test_properties_get_table_specific_property_that_doesnt_exist(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_get_table_does_not_exist(_):
runner = CliRunner()
- result = runner.invoke(run, ["properties", "get", "doesnotexist"])
+ result = runner.invoke(run, ["properties", "get", "table", "doesnotexist"])
assert result.exit_code == 1
- assert result.output == "Namespace does not exist: doesnotexist\n"
+ assert result.output == "Table does not exist: doesnotexist\n"
@mock.patch.dict(os.environ, MOCK_ENVIRONMENT)
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_get_namespace(_):
runner = CliRunner()
- result = runner.invoke(run, ["properties", "get", "default"])
+ result = runner.invoke(run, ["properties", "get", "namespace", "default"])
assert result.exit_code == 0
assert result.output == "location s3://warehouse/database/location\n"
@@ -402,11 +402,20 @@ def test_properties_get_namespace(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_get_namespace_specific_property(_):
runner = CliRunner()
- result = runner.invoke(run, ["properties", "get", "default", "location"])
+ result = runner.invoke(run, ["properties", "get", "namespace", "default", "location"])
assert result.exit_code == 0
assert result.output == "s3://warehouse/database/location\n"
+@mock.patch.dict(os.environ, MOCK_ENVIRONMENT)
+@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
+def test_properties_get_namespace_does_not_exist(_):
+ runner = CliRunner()
+ result = runner.invoke(run, ["properties", "get", "namespace", "doesnotexist"])
+ assert result.exit_code == 1
+ assert result.output == "Namespace does not exist: doesnotexist\n"
+
+
@mock.patch.dict(os.environ, MOCK_ENVIRONMENT)
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_properties_set_namespace(_):
@@ -683,7 +692,7 @@ def test_json_rename_table_does_not_exists(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_get_table(_):
runner = CliRunner()
- result = runner.invoke(run, ["--output=json", "properties", "get", "default.foo"])
+ result = runner.invoke(run, ["--output=json", "properties", "get", "table", "default.foo"])
assert result.exit_code == 0
assert result.output == """{"read.split.target.size": "134217728"}\n"""
@@ -692,7 +701,7 @@ def test_json_properties_get_table(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_get_table_specific_property(_):
runner = CliRunner()
- result = runner.invoke(run, ["--output=json", "properties", "get", "default.foo", "read.split.target.size"])
+ result = runner.invoke(run, ["--output=json", "properties", "get", "table", "default.foo", "read.split.target.size"])
assert result.exit_code == 0
assert result.output == """"134217728"\n"""
@@ -701,7 +710,7 @@ def test_json_properties_get_table_specific_property(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_get_table_specific_property_that_doesnt_exist(_):
runner = CliRunner()
- result = runner.invoke(run, ["--output=json", "properties", "get", "default.foo", "doesnotexist"])
+ result = runner.invoke(run, ["--output=json", "properties", "get", "table", "default.foo", "doesnotexist"])
assert result.exit_code == 1
assert (
result.output
@@ -713,16 +722,16 @@ def test_json_properties_get_table_specific_property_that_doesnt_exist(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_get_table_does_not_exist(_):
runner = CliRunner()
- result = runner.invoke(run, ["--output=json", "properties", "get", "doesnotexist"])
+ result = runner.invoke(run, ["--output=json", "properties", "get", "table", "doesnotexist"])
assert result.exit_code == 1
- assert result.output == """{"type": "NoSuchNamespaceError", "message": "Namespace does not exist: doesnotexist"}\n"""
+ assert result.output == """{"type": "NoSuchTableError", "message": "Table does not exist: doesnotexist"}\n"""
@mock.patch.dict(os.environ, MOCK_ENVIRONMENT)
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_get_namespace(_):
runner = CliRunner()
- result = runner.invoke(run, ["--output=json", "properties", "get", "default"])
+ result = runner.invoke(run, ["--output=json", "properties", "get", "namespace", "default"])
assert result.exit_code == 0
assert result.output == """{"location": "s3://warehouse/database/location"}\n"""
@@ -731,11 +740,20 @@ def test_json_properties_get_namespace(_):
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_get_namespace_specific_property(_):
runner = CliRunner()
- result = runner.invoke(run, ["--output=json", "properties", "get", "default", "location"])
+ result = runner.invoke(run, ["--output=json", "properties", "get", "namespace", "default", "location"])
assert result.exit_code == 0
assert result.output == """"s3://warehouse/database/location"\n"""
+@mock.patch.dict(os.environ, MOCK_ENVIRONMENT)
+@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
+def test_json_properties_get_namespace_does_not_exist(_):
+ runner = CliRunner()
+ result = runner.invoke(run, ["--output=json", "properties", "get", "namespace", "doesnotexist"])
+ assert result.exit_code == 1
+ assert result.output == """{"type": "NoSuchNamespaceError", "message": "Namespace does not exist: doesnotexist"}\n"""
+
+
@mock.patch.dict(os.environ, MOCK_ENVIRONMENT)
@mock.patch("pyiceberg.cli.console.load_catalog", return_value=MOCK_CATALOG)
def test_json_properties_set_namespace(_):