From 85b409df8ea26a7bcee22a1614ca8f0f4437994e Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Thu, 10 Nov 2022 13:27:40 -0800 Subject: [PATCH 01/13] cli support for setting recipient properties --- databricks_cli/unity_catalog/api.py | 4 ++-- .../unity_catalog/delta_sharing_cli.py | 24 +++++++++++++++---- databricks_cli/unity_catalog/uc_service.py | 6 ++++- tests/unity_catalog/test_delta_sharing_cli.py | 21 +++++++++++++--- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/databricks_cli/unity_catalog/api.py b/databricks_cli/unity_catalog/api.py index e9ec2876..af0cea84 100644 --- a/databricks_cli/unity_catalog/api.py +++ b/databricks_cli/unity_catalog/api.py @@ -193,8 +193,8 @@ def update_share_permissions(self, name, perm_spec): # Recipient APIs - def create_recipient(self, name, comment, sharing_id, allowed_ip_addresses): - return self.client.create_recipient(name, comment, sharing_id, allowed_ip_addresses) + def create_recipient(self, name, comment, sharing_id, allowed_ip_addresses, properties): + return self.client.create_recipient(name, comment, sharing_id, allowed_ip_addresses, properties) def list_recipients(self): return self.client.list_recipients() diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index 66ad374f..f9d60ce3 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -394,16 +394,22 @@ def delete_share_cli(api_client, name): help=( 'IP address in CIDR notation that is allowed to use delta sharing. ' '(can be specified multiple times).')) +@click.option('--property', default=None, type=JsonClickType(), required=False, multiple=True, + help=( + 'Custom properties of the recipient. For format should be ' + '{"key": "", "value": ""}. ' + 'Example: {"key": "country", "value": "US"}')) @debug_option @profile_option @eat_exceptions @provide_api_client -def create_recipient_cli(api_client, name, comment, sharing_id, allowed_ip_address): +def create_recipient_cli(api_client, name, comment, sharing_id, allowed_ip_address, property): """ Create a new recipient. """ + properties = [json_loads(property_str) for property_str in property] recipient_json = UnityCatalogApi(api_client).create_recipient( - name, comment, sharing_id, allowed_ip_address) + name, comment, sharing_id, allowed_ip_address, properties) click.echo(mc_pretty_format(recipient_json)) @@ -451,6 +457,12 @@ def get_recipient_cli(api_client, name): 'IP address in CIDR notation that is allowed to use delta sharing ' '(can be specified multiple times). Specify a single empty string to disable ' 'IP allowlist.')) +@click.option('--property', default=None, type=JsonClickType(), required=False, multiple=True, + help=( + 'Custom properties of the recipient. The format should be ' + '{"key": "", "value": ""}. ' + 'Example: {"key": "country", "value": "US"}. ' + 'Specify a single empty object `{}` to remove all properties.')) @click.option('--json-file', default=None, type=click.Path(), help=json_file_help(method='PATCH', path='/recipients/{name}')) @click.option('--json', default=None, type=JsonClickType(), @@ -460,14 +472,14 @@ def get_recipient_cli(api_client, name): @eat_exceptions @provide_api_client def update_recipient_cli(api_client, name, new_name, comment, owner, - allowed_ip_address, json_file, json): + allowed_ip_address, property, json_file, json): """ Update a recipient. The public specification for the JSON request is in development. """ if ((new_name is not None) or (comment is not None) or (owner is not None) or - len(allowed_ip_address) > 0): + len(allowed_ip_address) or len(property) > 0): if (json_file is not None) or (json is not None): raise ValueError('Cannot specify JSON if any other update flags are specified') data = {'name': new_name, 'comment': comment, 'owner': owner} @@ -475,6 +487,10 @@ def update_recipient_cli(api_client, name, new_name, comment, owner, data['ip_access_list'] = {} if len(allowed_ip_address) != 1 or allowed_ip_address[0] != '': data['ip_access_list']['allowed_ip_addresses'] = allowed_ip_address + if len(property) > 0: + data['properties_kvpairs'] = {} + if len(property) != 1 or json_loads(property[0]) != {}: + data['properties_kvpairs']['properties'] = [json_loads(property_str) for property_str in property] recipient_json = UnityCatalogApi(api_client).update_recipient(name, data) click.echo(mc_pretty_format(recipient_json)) else: diff --git a/databricks_cli/unity_catalog/uc_service.py b/databricks_cli/unity_catalog/uc_service.py index 92e4c9fa..0a842e0e 100644 --- a/databricks_cli/unity_catalog/uc_service.py +++ b/databricks_cli/unity_catalog/uc_service.py @@ -379,7 +379,7 @@ def update_share_permissions(self, name, perm_spec, headers=None): # Recipient Operations def create_recipient(self, name, comment=None, sharing_id=None, - allowed_ip_addresses=None, headers=None): + allowed_ip_addresses=None, properties = None, headers=None): _data = { 'name': name, } @@ -394,6 +394,10 @@ def create_recipient(self, name, comment=None, sharing_id=None, _data['ip_access_list'] = { 'allowed_ip_addresses': allowed_ip_addresses, } + if properties is not None: + _data['properties_kvpairs'] = { + 'properties': properties + } return self.client.perform_query('POST', '/unity-catalog/recipients', data=_data, headers=headers) diff --git a/tests/unity_catalog/test_delta_sharing_cli.py b/tests/unity_catalog/test_delta_sharing_cli.py index 4ad75d6d..ecfba2ee 100644 --- a/tests/unity_catalog/test_delta_sharing_cli.py +++ b/tests/unity_catalog/test_delta_sharing_cli.py @@ -463,9 +463,16 @@ def test_create_recipient_cli(api_mock, echo_mock): '--comment', 'comment', '--allowed-ip-address', '8.8.8.8', '--allowed-ip-address', '8.8.4.4', + '--property', '{"key": "k1", "value": "v1"}', + '--property', '{"key": "k2", "value": "v2"}' ]) api_mock.create_recipient.assert_called_once_with( - RECIPIENT_NAME, 'comment', None, ('8.8.8.8', '8.8.4.4')) + RECIPIENT_NAME, + 'comment', + None, + ('8.8.8.8', '8.8.4.4'), + [{"key": "k1", "value": "v1"}, {"key": "k2", "value": "v2"}] + ) echo_mock.assert_called_once_with(mc_pretty_format(RECIPIENT)) @@ -480,7 +487,7 @@ def test_create_recipient_cli_with_sharing_id(api_mock, echo_mock): '--sharing-id', '123e4567-e89b-12d3-a456-426614174000' ]) api_mock.create_recipient.assert_called_once_with( - RECIPIENT_NAME, None, '123e4567-e89b-12d3-a456-426614174000', ()) + RECIPIENT_NAME, None, '123e4567-e89b-12d3-a456-426614174000', (), []) echo_mock.assert_called_once_with(mc_pretty_format(RECIPIENT)) @@ -516,7 +523,9 @@ def test_update_recipient_cli(api_mock, echo_mock): '--comment', 'comment', '--owner', 'owner', '--allowed-ip-address', '8.8.8.8', - '--allowed-ip-address', '8.8.4.4' + '--allowed-ip-address', '8.8.4.4', + '--property', '{"key": "k1", "value": "v1"}', + '--property', '{"key": "k2", "value": "v2"}' ]) expected_data = { 'name': 'new_recipient_name', @@ -527,6 +536,12 @@ def test_update_recipient_cli(api_mock, echo_mock): '8.8.8.8', '8.8.4.4' ) + }, + 'properties_kvpairs': { + 'properties': [ + {"key": "k1", "value": "v1"}, + {"key": "k2", "value": "v2"} + ] } } api_mock.update_recipient.assert_called_once_with(RECIPIENT_NAME, expected_data) From d2dc3b286ab2a01c03eb5fca5c925b7123ce6889 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Thu, 10 Nov 2022 13:48:48 -0800 Subject: [PATCH 02/13] lint --- databricks_cli/unity_catalog/api.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/databricks_cli/unity_catalog/api.py b/databricks_cli/unity_catalog/api.py index af0cea84..3b3e47bc 100644 --- a/databricks_cli/unity_catalog/api.py +++ b/databricks_cli/unity_catalog/api.py @@ -193,8 +193,10 @@ def update_share_permissions(self, name, perm_spec): # Recipient APIs - def create_recipient(self, name, comment, sharing_id, allowed_ip_addresses, properties): - return self.client.create_recipient(name, comment, sharing_id, allowed_ip_addresses, properties) + def create_recipient(self, name, comment, sharing_id, + allowed_ip_addresses, properties): + return self.client.create_recipient(name, comment, sharing_id, + allowed_ip_addresses, properties) def list_recipients(self): return self.client.list_recipients() From b88761e38883984c1375c541763643be66fd6f5e Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Thu, 10 Nov 2022 17:48:53 -0800 Subject: [PATCH 03/13] update format of custom perperties --- databricks_cli/unity_catalog/api.py | 4 +- .../unity_catalog/delta_sharing_cli.py | 41 +++++++++++-------- databricks_cli/unity_catalog/uc_service.py | 6 +-- tests/unity_catalog/test_delta_sharing_cli.py | 8 ++-- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/databricks_cli/unity_catalog/api.py b/databricks_cli/unity_catalog/api.py index 3b3e47bc..6c3e513f 100644 --- a/databricks_cli/unity_catalog/api.py +++ b/databricks_cli/unity_catalog/api.py @@ -194,9 +194,9 @@ def update_share_permissions(self, name, perm_spec): # Recipient APIs def create_recipient(self, name, comment, sharing_id, - allowed_ip_addresses, properties): + allowed_ip_addresses, custom_properties): return self.client.create_recipient(name, comment, sharing_id, - allowed_ip_addresses, properties) + allowed_ip_addresses, custom_properties) def list_recipients(self): return self.client.list_recipients() diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index f9d60ce3..466bc70e 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -394,22 +394,25 @@ def delete_share_cli(api_client, name): help=( 'IP address in CIDR notation that is allowed to use delta sharing. ' '(can be specified multiple times).')) -@click.option('--property', default=None, type=JsonClickType(), required=False, multiple=True, +@click.option('--custom-property', default=None, required=False, multiple=True, help=( - 'Custom properties of the recipient. For format should be ' - '{"key": "", "value": ""}. ' - 'Example: {"key": "country", "value": "US"}')) + 'Custom properties of the recipient. Key and value should be provided ' + 'at the same time separated by an equal sign. ' + 'Example: --custom-property country=US')) @debug_option @profile_option @eat_exceptions @provide_api_client -def create_recipient_cli(api_client, name, comment, sharing_id, allowed_ip_address, property): +def create_recipient_cli(api_client, name, comment, sharing_id, + allowed_ip_address, custom_property): """ Create a new recipient. """ - properties = [json_loads(property_str) for property_str in property] + parsed_custom_properties = [property_str.split('=') for property_str in custom_property] + custom_properties = [{"key": property_key, "value": property_value} + for (property_key, property_value) in parsed_custom_properties] recipient_json = UnityCatalogApi(api_client).create_recipient( - name, comment, sharing_id, allowed_ip_address, properties) + name, comment, sharing_id, allowed_ip_address, custom_properties) click.echo(mc_pretty_format(recipient_json)) @@ -457,12 +460,12 @@ def get_recipient_cli(api_client, name): 'IP address in CIDR notation that is allowed to use delta sharing ' '(can be specified multiple times). Specify a single empty string to disable ' 'IP allowlist.')) -@click.option('--property', default=None, type=JsonClickType(), required=False, multiple=True, +@click.option('--custom-property', default=None, required=False, multiple=True, help=( - 'Custom properties of the recipient. The format should be ' - '{"key": "", "value": ""}. ' - 'Example: {"key": "country", "value": "US"}. ' - 'Specify a single empty object `{}` to remove all properties.')) + 'Custom properties of the recipient. Key and value should be provided ' + 'at the same time separated by an equal sign. ' + 'Example: --custom-property country=US' + 'Speficy a single empty string to remove all customer properties.')) @click.option('--json-file', default=None, type=click.Path(), help=json_file_help(method='PATCH', path='/recipients/{name}')) @click.option('--json', default=None, type=JsonClickType(), @@ -472,14 +475,14 @@ def get_recipient_cli(api_client, name): @eat_exceptions @provide_api_client def update_recipient_cli(api_client, name, new_name, comment, owner, - allowed_ip_address, property, json_file, json): + allowed_ip_address, custom_property, json_file, json): """ Update a recipient. The public specification for the JSON request is in development. """ if ((new_name is not None) or (comment is not None) or (owner is not None) or - len(allowed_ip_address) or len(property) > 0): + len(allowed_ip_address) or len(custom_property) > 0): if (json_file is not None) or (json is not None): raise ValueError('Cannot specify JSON if any other update flags are specified') data = {'name': new_name, 'comment': comment, 'owner': owner} @@ -487,10 +490,14 @@ def update_recipient_cli(api_client, name, new_name, comment, owner, data['ip_access_list'] = {} if len(allowed_ip_address) != 1 or allowed_ip_address[0] != '': data['ip_access_list']['allowed_ip_addresses'] = allowed_ip_address - if len(property) > 0: + if len(custom_property) > 0: data['properties_kvpairs'] = {} - if len(property) != 1 or json_loads(property[0]) != {}: - data['properties_kvpairs']['properties'] = [json_loads(property_str) for property_str in property] + if len(custom_property) != 1 or custom_property[0] != '': + parsed_custom_properties = [property_str.split('=') + for property_str in custom_property] + data['properties_kvpairs']['properties'] = [{"key": property_key, + "value": property_value} + for (property_key, property_value) in parsed_custom_properties] recipient_json = UnityCatalogApi(api_client).update_recipient(name, data) click.echo(mc_pretty_format(recipient_json)) else: diff --git a/databricks_cli/unity_catalog/uc_service.py b/databricks_cli/unity_catalog/uc_service.py index 0a842e0e..06a6c97b 100644 --- a/databricks_cli/unity_catalog/uc_service.py +++ b/databricks_cli/unity_catalog/uc_service.py @@ -379,7 +379,7 @@ def update_share_permissions(self, name, perm_spec, headers=None): # Recipient Operations def create_recipient(self, name, comment=None, sharing_id=None, - allowed_ip_addresses=None, properties = None, headers=None): + allowed_ip_addresses=None, custom_properties = None, headers=None): _data = { 'name': name, } @@ -394,9 +394,9 @@ def create_recipient(self, name, comment=None, sharing_id=None, _data['ip_access_list'] = { 'allowed_ip_addresses': allowed_ip_addresses, } - if properties is not None: + if custom_properties is not None: _data['properties_kvpairs'] = { - 'properties': properties + 'properties': custom_properties } return self.client.perform_query('POST', '/unity-catalog/recipients', data=_data, diff --git a/tests/unity_catalog/test_delta_sharing_cli.py b/tests/unity_catalog/test_delta_sharing_cli.py index ecfba2ee..35dcef5e 100644 --- a/tests/unity_catalog/test_delta_sharing_cli.py +++ b/tests/unity_catalog/test_delta_sharing_cli.py @@ -463,8 +463,8 @@ def test_create_recipient_cli(api_mock, echo_mock): '--comment', 'comment', '--allowed-ip-address', '8.8.8.8', '--allowed-ip-address', '8.8.4.4', - '--property', '{"key": "k1", "value": "v1"}', - '--property', '{"key": "k2", "value": "v2"}' + '--custom-property', 'k1=v1', + '--custom-property', 'k2=v2' ]) api_mock.create_recipient.assert_called_once_with( RECIPIENT_NAME, @@ -524,8 +524,8 @@ def test_update_recipient_cli(api_mock, echo_mock): '--owner', 'owner', '--allowed-ip-address', '8.8.8.8', '--allowed-ip-address', '8.8.4.4', - '--property', '{"key": "k1", "value": "v1"}', - '--property', '{"key": "k2", "value": "v2"}' + '--custom-property', 'k1=v1', + '--custom-property', 'k2=v2' ]) expected_data = { 'name': 'new_recipient_name', From 7b39b12f0ec53f1944b731cb97bb64cfc2fe32e0 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Thu, 10 Nov 2022 17:51:24 -0800 Subject: [PATCH 04/13] lint --- databricks_cli/unity_catalog/uc_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks_cli/unity_catalog/uc_service.py b/databricks_cli/unity_catalog/uc_service.py index 06a6c97b..04eec1a5 100644 --- a/databricks_cli/unity_catalog/uc_service.py +++ b/databricks_cli/unity_catalog/uc_service.py @@ -379,7 +379,7 @@ def update_share_permissions(self, name, perm_spec, headers=None): # Recipient Operations def create_recipient(self, name, comment=None, sharing_id=None, - allowed_ip_addresses=None, custom_properties = None, headers=None): + allowed_ip_addresses=None, custom_properties=None, headers=None): _data = { 'name': name, } From 674def3742426d51d929a1f52a116e02cb1258d8 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Fri, 11 Nov 2022 10:30:20 -0800 Subject: [PATCH 05/13] add max split --- databricks_cli/unity_catalog/delta_sharing_cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index 466bc70e..fe06609b 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -408,7 +408,7 @@ def create_recipient_cli(api_client, name, comment, sharing_id, """ Create a new recipient. """ - parsed_custom_properties = [property_str.split('=') for property_str in custom_property] + parsed_custom_properties = [property_str.split('=', 2) for property_str in custom_property] custom_properties = [{"key": property_key, "value": property_value} for (property_key, property_value) in parsed_custom_properties] recipient_json = UnityCatalogApi(api_client).create_recipient( @@ -493,7 +493,7 @@ def update_recipient_cli(api_client, name, new_name, comment, owner, if len(custom_property) > 0: data['properties_kvpairs'] = {} if len(custom_property) != 1 or custom_property[0] != '': - parsed_custom_properties = [property_str.split('=') + parsed_custom_properties = [property_str.split('=', 2) for property_str in custom_property] data['properties_kvpairs']['properties'] = [{"key": property_key, "value": property_value} From 0237afc26d42fffeff8393bdcb985a9c6c57b5d9 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Fri, 11 Nov 2022 10:35:55 -0800 Subject: [PATCH 06/13] add test for invalid property --- tests/unity_catalog/test_delta_sharing_cli.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unity_catalog/test_delta_sharing_cli.py b/tests/unity_catalog/test_delta_sharing_cli.py index 35dcef5e..f9e677a9 100644 --- a/tests/unity_catalog/test_delta_sharing_cli.py +++ b/tests/unity_catalog/test_delta_sharing_cli.py @@ -476,6 +476,20 @@ def test_create_recipient_cli(api_mock, echo_mock): echo_mock.assert_called_once_with(mc_pretty_format(RECIPIENT)) +@provide_conf +def test_create_recipient_cli_invalid_custom_property(api_mock, echo_mock): + api_mock.create_recipient.return_value = RECIPIENT + runner = CliRunner() + runner.invoke( + delta_sharing_cli.create_recipient_cli, + args=[ + '--name', RECIPIENT_NAME, + '--custom-property', 'k1=v1=v2' + ]) + + assert not api_mock.create_recipient.called + + @provide_conf def test_create_recipient_cli_with_sharing_id(api_mock, echo_mock): api_mock.create_recipient.return_value = RECIPIENT From c88386da6be916ab22c9373f20aad7f50561fc27 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Fri, 11 Nov 2022 10:40:19 -0800 Subject: [PATCH 07/13] lint --- tests/unity_catalog/test_delta_sharing_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unity_catalog/test_delta_sharing_cli.py b/tests/unity_catalog/test_delta_sharing_cli.py index f9e677a9..fdec3b5b 100644 --- a/tests/unity_catalog/test_delta_sharing_cli.py +++ b/tests/unity_catalog/test_delta_sharing_cli.py @@ -477,7 +477,7 @@ def test_create_recipient_cli(api_mock, echo_mock): @provide_conf -def test_create_recipient_cli_invalid_custom_property(api_mock, echo_mock): +def test_create_recipient_cli_invalid_custom_property(api_mock): api_mock.create_recipient.return_value = RECIPIENT runner = CliRunner() runner.invoke( From 01e940fb1d37cea6c9d800e4babce53124073e83 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Fri, 11 Nov 2022 13:31:30 -0800 Subject: [PATCH 08/13] add more validation --- .../unity_catalog/delta_sharing_cli.py | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index fe06609b..786675f7 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -408,9 +408,13 @@ def create_recipient_cli(api_client, name, comment, sharing_id, """ Create a new recipient. """ - parsed_custom_properties = [property_str.split('=', 2) for property_str in custom_property] - custom_properties = [{"key": property_key, "value": property_value} - for (property_key, property_value) in parsed_custom_properties] + custom_properties = [] + for property_str in custom_property: + tokens = property_str.split('=', 2) + if len(tokens) != 2: + raise ValueError('Invalid format of custom property. ' + + 'The format should be =.') + custom_properties.append({"key": tokens[0], "value": tokens[1]}) recipient_json = UnityCatalogApi(api_client).create_recipient( name, comment, sharing_id, allowed_ip_address, custom_properties) click.echo(mc_pretty_format(recipient_json)) @@ -465,7 +469,7 @@ def get_recipient_cli(api_client, name): 'Custom properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' 'Example: --custom-property country=US' - 'Speficy a single empty string to remove all customer properties.')) + 'Specify a single empty string to remove all custom properties.')) @click.option('--json-file', default=None, type=click.Path(), help=json_file_help(method='PATCH', path='/recipients/{name}')) @click.option('--json', default=None, type=JsonClickType(), @@ -493,11 +497,14 @@ def update_recipient_cli(api_client, name, new_name, comment, owner, if len(custom_property) > 0: data['properties_kvpairs'] = {} if len(custom_property) != 1 or custom_property[0] != '': - parsed_custom_properties = [property_str.split('=', 2) - for property_str in custom_property] - data['properties_kvpairs']['properties'] = [{"key": property_key, - "value": property_value} - for (property_key, property_value) in parsed_custom_properties] + data['properties_kvpairs']['properties'] = [] + for property_str in custom_property: + tokens = property_str.split('=', 2) + if len(tokens) != 2: + raise ValueError('Invalid format of custom property. ' + + 'The format should be =.') + data['properties_kvpairs']['properties'].append({"key": tokens[0], + "value": tokens[1]}) recipient_json = UnityCatalogApi(api_client).update_recipient(name, data) click.echo(mc_pretty_format(recipient_json)) else: From b69d429725e1d5df62f51878db33228b0b241d40 Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Tue, 15 Nov 2022 10:23:05 -0800 Subject: [PATCH 09/13] address comments --- .../unity_catalog/delta_sharing_cli.py | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index 786675f7..ba6bec05 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -383,6 +383,17 @@ def delete_share_cli(api_client, name): ############## Recipient Commands ############## +def parse_recipient_custom_properties(custom_property_list): + custom_properties = [] + for property_str in custom_property_list: + tokens = property_str.split('=', 2) + if len(tokens) != 2: + raise ValueError('Invalid format of custom property. ' + + 'The format should be =.') + custom_properties.append({"key": tokens[0], "value": tokens[1]}) + return custom_properties + + @click.command(context_settings=CONTEXT_SETTINGS, short_help='Create a new recipient.') @click.option('--name', required=True, help='Name of new recipient.') @@ -398,7 +409,7 @@ def delete_share_cli(api_client, name): help=( 'Custom properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' - 'Example: --custom-property country=US')) + 'Example: --custom-property country=US.')) @debug_option @profile_option @eat_exceptions @@ -408,15 +419,9 @@ def create_recipient_cli(api_client, name, comment, sharing_id, """ Create a new recipient. """ - custom_properties = [] - for property_str in custom_property: - tokens = property_str.split('=', 2) - if len(tokens) != 2: - raise ValueError('Invalid format of custom property. ' - + 'The format should be =.') - custom_properties.append({"key": tokens[0], "value": tokens[1]}) recipient_json = UnityCatalogApi(api_client).create_recipient( - name, comment, sharing_id, allowed_ip_address, custom_properties) + name, comment, sharing_id, + allowed_ip_address, parse_recipient_custom_properties(custom_property)) click.echo(mc_pretty_format(recipient_json)) @@ -468,7 +473,7 @@ def get_recipient_cli(api_client, name): help=( 'Custom properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' - 'Example: --custom-property country=US' + 'Example: --custom-property country=US. ' 'Specify a single empty string to remove all custom properties.')) @click.option('--json-file', default=None, type=click.Path(), help=json_file_help(method='PATCH', path='/recipients/{name}')) @@ -497,14 +502,8 @@ def update_recipient_cli(api_client, name, new_name, comment, owner, if len(custom_property) > 0: data['properties_kvpairs'] = {} if len(custom_property) != 1 or custom_property[0] != '': - data['properties_kvpairs']['properties'] = [] - for property_str in custom_property: - tokens = property_str.split('=', 2) - if len(tokens) != 2: - raise ValueError('Invalid format of custom property. ' - + 'The format should be =.') - data['properties_kvpairs']['properties'].append({"key": tokens[0], - "value": tokens[1]}) + data['properties_kvpairs']['properties'] = parse_recipient_custom_properties( + custom_property) recipient_json = UnityCatalogApi(api_client).update_recipient(name, data) click.echo(mc_pretty_format(recipient_json)) else: From 97b9961d2cf0071e5fb834d6c0ba2ed80e43f51e Mon Sep 17 00:00:00 2001 From: Xiaotong Sun Date: Tue, 15 Nov 2022 10:49:18 -0800 Subject: [PATCH 10/13] rename public argument name --- databricks_cli/unity_catalog/delta_sharing_cli.py | 4 ++-- tests/unity_catalog/test_delta_sharing_cli.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index ba6bec05..baf4e25e 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -405,7 +405,7 @@ def parse_recipient_custom_properties(custom_property_list): help=( 'IP address in CIDR notation that is allowed to use delta sharing. ' '(can be specified multiple times).')) -@click.option('--custom-property', default=None, required=False, multiple=True, +@click.option('--property', 'custom_property', default=None, required=False, multiple=True, help=( 'Custom properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' @@ -469,7 +469,7 @@ def get_recipient_cli(api_client, name): 'IP address in CIDR notation that is allowed to use delta sharing ' '(can be specified multiple times). Specify a single empty string to disable ' 'IP allowlist.')) -@click.option('--custom-property', default=None, required=False, multiple=True, +@click.option('--property', 'custom_property', default=None, required=False, multiple=True, help=( 'Custom properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' diff --git a/tests/unity_catalog/test_delta_sharing_cli.py b/tests/unity_catalog/test_delta_sharing_cli.py index fdec3b5b..7e0dbb5b 100644 --- a/tests/unity_catalog/test_delta_sharing_cli.py +++ b/tests/unity_catalog/test_delta_sharing_cli.py @@ -463,8 +463,8 @@ def test_create_recipient_cli(api_mock, echo_mock): '--comment', 'comment', '--allowed-ip-address', '8.8.8.8', '--allowed-ip-address', '8.8.4.4', - '--custom-property', 'k1=v1', - '--custom-property', 'k2=v2' + '--property', 'k1=v1', + '--property', 'k2=v2' ]) api_mock.create_recipient.assert_called_once_with( RECIPIENT_NAME, @@ -484,7 +484,7 @@ def test_create_recipient_cli_invalid_custom_property(api_mock): delta_sharing_cli.create_recipient_cli, args=[ '--name', RECIPIENT_NAME, - '--custom-property', 'k1=v1=v2' + '--property', 'k1=v1=v2' ]) assert not api_mock.create_recipient.called @@ -538,8 +538,8 @@ def test_update_recipient_cli(api_mock, echo_mock): '--owner', 'owner', '--allowed-ip-address', '8.8.8.8', '--allowed-ip-address', '8.8.4.4', - '--custom-property', 'k1=v1', - '--custom-property', 'k2=v2' + '--property', 'k1=v1', + '--property', 'k2=v2' ]) expected_data = { 'name': 'new_recipient_name', From e5bdd6992127128b2ae193f7c6ddc6964b084dfd Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 16 Nov 2022 08:59:45 +0100 Subject: [PATCH 11/13] Update databricks_cli/unity_catalog/delta_sharing_cli.py --- databricks_cli/unity_catalog/delta_sharing_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index baf4e25e..4e7e2cc3 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -388,7 +388,7 @@ def parse_recipient_custom_properties(custom_property_list): for property_str in custom_property_list: tokens = property_str.split('=', 2) if len(tokens) != 2: - raise ValueError('Invalid format of custom property. ' + raise ValueError('Invalid format for property. ' + 'The format should be =.') custom_properties.append({"key": tokens[0], "value": tokens[1]}) return custom_properties From 6579764f81433093637420f89c23373a6d5ad6a5 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 16 Nov 2022 08:59:53 +0100 Subject: [PATCH 12/13] Update databricks_cli/unity_catalog/delta_sharing_cli.py --- databricks_cli/unity_catalog/delta_sharing_cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index 4e7e2cc3..84e82dc1 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -407,9 +407,9 @@ def parse_recipient_custom_properties(custom_property_list): '(can be specified multiple times).')) @click.option('--property', 'custom_property', default=None, required=False, multiple=True, help=( - 'Custom properties of the recipient. Key and value should be provided ' + 'Properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' - 'Example: --custom-property country=US.')) + 'Example: --property country=US.')) @debug_option @profile_option @eat_exceptions From 24698ddc5420eae27d6b4862e3834f2ae6458e57 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 16 Nov 2022 08:59:58 +0100 Subject: [PATCH 13/13] Update databricks_cli/unity_catalog/delta_sharing_cli.py --- databricks_cli/unity_catalog/delta_sharing_cli.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/databricks_cli/unity_catalog/delta_sharing_cli.py b/databricks_cli/unity_catalog/delta_sharing_cli.py index 84e82dc1..988c5fbb 100644 --- a/databricks_cli/unity_catalog/delta_sharing_cli.py +++ b/databricks_cli/unity_catalog/delta_sharing_cli.py @@ -471,10 +471,10 @@ def get_recipient_cli(api_client, name): 'IP allowlist.')) @click.option('--property', 'custom_property', default=None, required=False, multiple=True, help=( - 'Custom properties of the recipient. Key and value should be provided ' + 'Properties of the recipient. Key and value should be provided ' 'at the same time separated by an equal sign. ' - 'Example: --custom-property country=US. ' - 'Specify a single empty string to remove all custom properties.')) + 'Example: --property country=US. ' + 'Specify a single empty string to remove all properties.')) @click.option('--json-file', default=None, type=click.Path(), help=json_file_help(method='PATCH', path='/recipients/{name}')) @click.option('--json', default=None, type=JsonClickType(),