Skip to content

Commit 68fb898

Browse files
authored
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.
1 parent 2d3fb01 commit 68fb898

File tree

3 files changed

+20
-1
lines changed

3 files changed

+20
-1
lines changed

regtests/client/python/cli/options/parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ def parse_properties(properties: List[str]) -> Optional[Dict[str, str]]:
104104
if '=' not in property:
105105
raise Exception(f'Could not parse property `{property}`')
106106
key, value = property.split('=', 1)
107-
if '=' in value or not value:
107+
if not value:
108108
raise Exception(f'Could not parse property `{property}`')
109109
if key in results:
110110
raise Exception(f'Duplicate property key `{key}`')

regtests/client/python/test/test_cli_parsing.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,11 @@ def get(obj, arg_string):
273273
'get_catalog', {
274274
(0, None): 'foo',
275275
})
276+
check_arguments(
277+
mock_execute(['catalogs', 'update', 'foo', '--set-property', 'listkey=k1=v1,k2=v2']),
278+
'get_catalog', {
279+
(0, None): 'foo',
280+
})
276281
check_arguments(
277282
mock_execute(['catalogs', 'update', 'foo', '--remove-property', 'key']),
278283
'get_catalog', {

regtests/t_cli/src/test_cli.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,20 @@ def test_update_catalog():
373373
checker=lambda s: 'foo' not in s and 'prop2' not in s and
374374
'"prop3": "3333"' in s and '"prop4": "4444"' in s)
375375

376+
# Update to add a property whose value is a key-value list
377+
check_output(root_cli(
378+
'catalogs',
379+
'update',
380+
f'test_cli_catalog_{SALT}',
381+
'--set-property',
382+
'listprop=k1=v1,k2=v2'
383+
), checker=lambda s: s == '')
384+
385+
# Previous properties still exist, and the new property is parsed properly
386+
check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
387+
checker=lambda s: '"prop3": "3333"' in s and '"prop4": "4444"' in s and
388+
'"listprop": "k1=v1,k2=v2"' in s)
389+
376390
# Update to set a region
377391
check_output(root_cli(
378392
'catalogs',

0 commit comments

Comments
 (0)