Skip to content

Commit af337ba

Browse files
authored
fix: preserve empty property values in CLI (#3745)
<!-- Thanks for opening a pull request! --> # Rationale for this change The `properties get table` and `properties get namespace` commands used a truthy check for property lookups. As a result, an existing property with an empty string value was reported as missing. This changes both lookups to check explicitly for `None`, preserving empty strings while keeping the existing error for missing properties. Related to #3713. ## Are these changes tested? Yes. Added CLI regression tests for empty table and namespace property values. - `make lint` - `uv run python -m pytest tests/cli/test_console.py` ## Are there any user-facing changes? Yes. The table and namespace property commands now return an existing empty string value instead of raising `NoSuchPropertyException`. <!-- In the case of user-facing changes, please add the changelog label. -->
1 parent 07e5791 commit af337ba

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

pyiceberg/cli/console.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,8 @@ def get_namespace(ctx: Context, identifier: str, property_name: str) -> None:
327327
namespace_properties = catalog.load_namespace_properties(identifier_tuple)
328328

329329
if property_name:
330-
if property_value := namespace_properties.get(property_name):
330+
property_value = namespace_properties.get(property_name)
331+
if property_value is not None:
331332
output.text(property_value)
332333
else:
333334
raise NoSuchPropertyException(f"Could not find property {property_name} on namespace {identifier}")
@@ -348,7 +349,8 @@ def get_table(ctx: Context, identifier: str, property_name: str) -> None:
348349
metadata = catalog.load_table(identifier_tuple).metadata
349350

350351
if property_name:
351-
if property_value := metadata.properties.get(property_name):
352+
property_value = metadata.properties.get(property_name)
353+
if property_value is not None:
352354
output.text(property_value)
353355
else:
354356
raise NoSuchPropertyException(f"Could not find property {property_name} on table {identifier}")

tests/cli/test_console.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,21 @@ def test_properties_get_table_specific_property(catalog: InMemoryCatalog) -> Non
440440
assert result.output == "134217728\n"
441441

442442

443+
def test_properties_get_table_specific_empty_property(catalog: InMemoryCatalog) -> None:
444+
catalog.create_namespace(TEST_TABLE_NAMESPACE)
445+
catalog.create_table(
446+
identifier=TEST_TABLE_IDENTIFIER,
447+
schema=TEST_TABLE_SCHEMA,
448+
partition_spec=TEST_TABLE_PARTITION_SPEC,
449+
properties={"empty": ""},
450+
)
451+
452+
runner = CliRunner()
453+
result = runner.invoke(run, ["properties", "get", "table", "default.my_table", "empty"])
454+
assert result.exit_code == 0
455+
assert result.output == "\n"
456+
457+
443458
def test_properties_get_table_specific_property_that_doesnt_exist(catalog: InMemoryCatalog) -> None:
444459
catalog.create_namespace(TEST_TABLE_NAMESPACE)
445460
catalog.create_table(
@@ -482,6 +497,15 @@ def test_properties_get_namespace_specific_property(catalog: InMemoryCatalog, na
482497
assert result.output == "s3://warehouse/database/location\n"
483498

484499

500+
def test_properties_get_namespace_specific_empty_property(catalog: InMemoryCatalog) -> None:
501+
catalog.create_namespace(TEST_TABLE_NAMESPACE, {"empty": ""})
502+
503+
runner = CliRunner()
504+
result = runner.invoke(run, ["properties", "get", "namespace", "default", "empty"])
505+
assert result.exit_code == 0
506+
assert result.output == "\n"
507+
508+
485509
def test_properties_get_namespace_does_not_exist(catalog: InMemoryCatalog, namespace_properties: Properties) -> None:
486510
catalog.create_namespace(TEST_TABLE_NAMESPACE, namespace_properties)
487511

0 commit comments

Comments
 (0)