This is an automated email from the ASF dual-hosted git repository.
dhuo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 68fb898d Allow Polaris CLI to set property values that contain '='
(#1003)
68fb898d is described below
commit 68fb898dfd9950cb312a26e923ab0f4e4f0cb6dc
Author: Dennis Huo <[email protected]>
AuthorDate: Fri Feb 21 18:49:53 2025 -0800
Allow Polaris CLI to set property values that contain '=' (#1003)
Currently the CLI parsing is too pessimistic and throws an error if
it detects '=' in the parsed value of a property. However, the split 1
semantics are already standard behavior for most parsers where the
right-hand-side is allowed to contain '='.
This commonly comes up if the value is itself a comma-separated kv list
or if it the value is a base64-encoded string.
---
regtests/client/python/cli/options/parser.py | 2 +-
regtests/client/python/test/test_cli_parsing.py | 5 +++++
regtests/t_cli/src/test_cli.py | 14 ++++++++++++++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/regtests/client/python/cli/options/parser.py
b/regtests/client/python/cli/options/parser.py
index 4a26a758..9b91e561 100644
--- a/regtests/client/python/cli/options/parser.py
+++ b/regtests/client/python/cli/options/parser.py
@@ -104,7 +104,7 @@ class Parser(object):
if '=' not in property:
raise Exception(f'Could not parse property `{property}`')
key, value = property.split('=', 1)
- if '=' in value or not value:
+ if not value:
raise Exception(f'Could not parse property `{property}`')
if key in results:
raise Exception(f'Duplicate property key `{key}`')
diff --git a/regtests/client/python/test/test_cli_parsing.py
b/regtests/client/python/test/test_cli_parsing.py
index b9967ce9..65e078b1 100644
--- a/regtests/client/python/test/test_cli_parsing.py
+++ b/regtests/client/python/test/test_cli_parsing.py
@@ -273,6 +273,11 @@ class TestCliParsing(unittest.TestCase):
'get_catalog', {
(0, None): 'foo',
})
+ check_arguments(
+ mock_execute(['catalogs', 'update', 'foo', '--set-property',
'listkey=k1=v1,k2=v2']),
+ 'get_catalog', {
+ (0, None): 'foo',
+ })
check_arguments(
mock_execute(['catalogs', 'update', 'foo', '--remove-property',
'key']),
'get_catalog', {
diff --git a/regtests/t_cli/src/test_cli.py b/regtests/t_cli/src/test_cli.py
index 02071875..da28bb57 100644
--- a/regtests/t_cli/src/test_cli.py
+++ b/regtests/t_cli/src/test_cli.py
@@ -373,6 +373,20 @@ def test_update_catalog():
checker=lambda s: 'foo' not in s and 'prop2' not in s and
'"prop3": "3333"' in s and '"prop4":
"4444"' in s)
+ # Update to add a property whose value is a key-value list
+ check_output(root_cli(
+ 'catalogs',
+ 'update',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'listprop=k1=v1,k2=v2'
+ ), checker=lambda s: s == '')
+
+ # Previous properties still exist, and the new property is parsed
properly
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: '"prop3": "3333"' in s and '"prop4":
"4444"' in s and
+ '"listprop": "k1=v1,k2=v2"' in s)
+
# Update to set a region
check_output(root_cli(
'catalogs',
