You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by em...@apache.org on 2024/10/30 20:22:08 UTC
(polaris) branch main updated: Fix polaris CLI support for updating catalog properties and hitting real Polaris services (#404)
This is an automated email from the ASF dual-hosted git repository.
emaynard 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 ea7ffebb Fix polaris CLI support for updating catalog properties and hitting real Polaris services (#404)
ea7ffebb is described below
commit ea7ffebbef53061c840bf9524ef581d46295120c
Author: Dennis Huo <74...@users.noreply.github.com>
AuthorDate: Wed Oct 30 13:22:04 2024 -0700
Fix polaris CLI support for updating catalog properties and hitting real Polaris services (#404)
* Fix polaris CLI support for updating catalog properties and hitting real Polaris services
Add a --base-url argument that can be used instead of --host --port to support
arbitrary Polaris URLs including https ones that have additional base prefixes
before the /api/*/v1 subpaths.
Fix namespaces.py to parse such URLs as well.
Fix `catalogs update` to explicitly preserve default-base-location in the update
request and use dict merge semantics for specified properties instead of just
replacing the entire catalog contents with only what's specified in cli arguments,
otherwise default-base-location or other properties get clobbered unexpectedly.
Make the test_cli.py abide by POLARIS_HOST so that it can be pointed at a basic
`./gradlew runApp` with `POLARIS_HOST=localhost` even without a docker container.
Add tests for updating catalog properties.
* Small whitespace consistency fix.
* Also update test_cli_parsing
Add checks for not specifying both --base-url and --host/--port
Remove duplicate examples
* Introduce --set-property and --remove-property for all "update" commands instead of using --property.
This pattern matches Iceberg Spark's behavior for e.g. ALTER TABLE SET TBLPROPERTIES and ALTER TABLE UNSET TBLPROPERTIES
where the SET command has "merge" semantics and thus UNSET is analogous to the separate --remove-property. This
syntax is also consistent with other cloud providers' command-line tools such as `gcloud compute instances update --update-labels ... --remove-labels
Implement consistent merge/remove pattern for catalogs/principals/principal-roles/catalog-roles
Fix `namespaces create` to correctly merge --location with --property
Fix `namespaces get` to print the full response json after calling the right load_namespace_metadata API instead of only calling namespace_exists
Fix the handling of --base-url being exclusive to --host/--port
* Also add --remove-property to cli parsing test
* Also test parsing invalid --set-property on create
---
regtests/client/python/cli/command/__init__.py | 18 +-
.../client/python/cli/command/catalog_roles.py | 17 +-
regtests/client/python/cli/command/catalogs.py | 32 +-
regtests/client/python/cli/command/namespaces.py | 13 +-
.../client/python/cli/command/principal_roles.py | 17 +-
regtests/client/python/cli/command/principals.py | 19 +-
regtests/client/python/cli/constants.py | 12 +
regtests/client/python/cli/options/option_tree.py | 12 +-
regtests/client/python/cli/options/parser.py | 5 +-
regtests/client/python/cli/polaris_cli.py | 26 +-
regtests/client/python/test/test_cli_parsing.py | 47 ++-
regtests/t_cli/src/test_cli.py | 383 ++++++++++++++++++++-
12 files changed, 554 insertions(+), 47 deletions(-)
diff --git a/regtests/client/python/cli/command/__init__.py b/regtests/client/python/cli/command/__init__.py
index c0289fba..9f877eba 100644
--- a/regtests/client/python/cli/command/__init__.py
+++ b/regtests/client/python/cli/command/__init__.py
@@ -38,6 +38,8 @@ class Command(ABC):
return f(getattr(options, key)) if hasattr(options, key) else None
properties = Parser.parse_properties(options_get(Arguments.PROPERTY))
+ set_properties = Parser.parse_properties(options_get(Arguments.SET_PROPERTY))
+ remove_properties = options_get(Arguments.REMOVE_PROPERTY)
command = None
if options.command == Commands.CATALOGS:
@@ -57,7 +59,9 @@ class Command(ABC):
consent_url=options_get(Arguments.CONSENT_URL),
service_account=options_get(Arguments.SERVICE_ACCOUNT),
catalog_name=options_get(Arguments.CATALOG),
- properties={} if properties is None else properties
+ properties={} if properties is None else properties,
+ set_properties={} if set_properties is None else set_properties,
+ remove_properties=[] if remove_properties is None else remove_properties
)
elif options.command == Commands.PRINCIPALS:
from cli.command.principals import PrincipalsCommand
@@ -67,7 +71,9 @@ class Command(ABC):
principal_name=options_get(Arguments.PRINCIPAL),
client_id=options_get(Arguments.CLIENT_ID),
principal_role=options_get(Arguments.PRINCIPAL_ROLE),
- properties=properties
+ properties={} if properties is None else properties,
+ set_properties={} if set_properties is None else set_properties,
+ remove_properties=[] if remove_properties is None else remove_properties
)
elif options.command == Commands.PRINCIPAL_ROLES:
from cli.command.principal_roles import PrincipalRolesCommand
@@ -77,7 +83,9 @@ class Command(ABC):
principal_name=options_get(Arguments.PRINCIPAL),
catalog_name=options_get(Arguments.CATALOG),
catalog_role_name=options_get(Arguments.CATALOG_ROLE),
- properties=properties
+ properties={} if properties is None else properties,
+ set_properties={} if set_properties is None else set_properties,
+ remove_properties=[] if remove_properties is None else remove_properties
)
elif options.command == Commands.CATALOG_ROLES:
from cli.command.catalog_roles import CatalogRolesCommand
@@ -86,7 +94,9 @@ class Command(ABC):
catalog_name=options_get(Arguments.CATALOG),
catalog_role_name=options_get(Arguments.CATALOG_ROLE),
principal_role_name=options_get(Arguments.PRINCIPAL_ROLE),
- properties=properties
+ properties={} if properties is None else properties,
+ set_properties={} if set_properties is None else set_properties,
+ remove_properties=[] if remove_properties is None else remove_properties
)
elif options.command == Commands.PRIVILEGES:
from cli.command.privileges import PrivilegesCommand
diff --git a/regtests/client/python/cli/command/catalog_roles.py b/regtests/client/python/cli/command/catalog_roles.py
index a9b30d6e..1c7d4df9 100644
--- a/regtests/client/python/cli/command/catalog_roles.py
+++ b/regtests/client/python/cli/command/catalog_roles.py
@@ -17,7 +17,7 @@
# under the License.
#
from dataclasses import dataclass
-from typing import Dict, Optional
+from typing import Dict, Optional, List
from pydantic import StrictStr
@@ -46,6 +46,8 @@ class CatalogRolesCommand(Command):
catalog_role_name: str
principal_role_name: str
properties: Optional[Dict[str, StrictStr]]
+ set_properties: Dict[str, StrictStr]
+ remove_properties: List[str]
def validate(self):
if not self.catalog_name:
@@ -77,9 +79,20 @@ class CatalogRolesCommand(Command):
print(catalog_role.to_json())
elif self.catalog_roles_subcommand == Subcommands.UPDATE:
catalog_role = api.get_catalog_role(self.catalog_name, self.catalog_role_name)
+ new_properties = catalog_role.properties or {}
+
+ # Add or update all entries specified in set_properties
+ if self.set_properties:
+ new_properties = {**new_properties, **self.set_properties}
+
+ # Remove all keys specified in remove_properties
+ if self.remove_properties:
+ for to_remove in self.remove_properties:
+ new_properties.pop(to_remove, None)
+
request = UpdateCatalogRoleRequest(
current_entity_version=catalog_role.entity_version,
- properties=self.properties
+ properties=new_properties
)
api.update_catalog_role(self.catalog_name, self.catalog_role_name, request)
elif self.catalog_roles_subcommand == Subcommands.GRANT:
diff --git a/regtests/client/python/cli/command/catalogs.py b/regtests/client/python/cli/command/catalogs.py
index cbe12e28..ceef7855 100644
--- a/regtests/client/python/cli/command/catalogs.py
+++ b/regtests/client/python/cli/command/catalogs.py
@@ -57,6 +57,8 @@ class CatalogsCommand(Command):
service_account: str
catalog_name: str
properties: Dict[str, StrictStr]
+ set_properties: Dict[str, StrictStr]
+ remove_properties: List[str]
def validate(self):
if self.catalogs_subcommand == Subcommands.CREATE:
@@ -176,22 +178,36 @@ class CatalogsCommand(Command):
print(catalog.to_json())
elif self.catalogs_subcommand == Subcommands.UPDATE:
catalog = api.get_catalog(self.catalog_name)
- if self.default_base_location or self.properties:
+
+ if self.default_base_location or self.set_properties or self.remove_properties:
+ new_default_base_location = self.default_base_location or catalog.properties.default_base_location
+ new_additional_properties = catalog.properties.additional_properties or {}
+
+ # Add or update all entries specified in set_properties
+ if self.set_properties:
+ new_additional_properties = {**new_additional_properties, **self.set_properties}
+
+ # Remove all keys specified in remove_properties
+ if self.remove_properties:
+ for to_remove in self.remove_properties:
+ new_additional_properties.pop(to_remove, None)
+
catalog.properties = CatalogProperties(
- default_base_location=self.default_base_location,
- additional_properties=self.properties
+ default_base_location=new_default_base_location,
+ additional_properties=new_additional_properties
)
- request = UpdateCatalogRequest(
- current_entity_version=catalog.entity_version,
- catalog=catalog
- )
if (self._has_aws_storage_info() or self._has_azure_storage_info() or self._has_gcs_storage_info() or
self.allowed_locations or self.default_base_location):
request = UpdateCatalogRequest(
current_entity_version=catalog.entity_version,
- catalog=catalog,
+ properties=catalog.properties.to_dict(),
storage_config_info=self._build_storage_config_info()
)
+ else:
+ request = UpdateCatalogRequest(
+ current_entity_version=catalog.entity_version,
+ properties=catalog.properties.to_dict()
+ )
api.update_catalog(self.catalog_name, request)
else:
diff --git a/regtests/client/python/cli/command/namespaces.py b/regtests/client/python/cli/command/namespaces.py
index 472b14f9..313d4b0d 100644
--- a/regtests/client/python/cli/command/namespaces.py
+++ b/regtests/client/python/cli/command/namespaces.py
@@ -59,7 +59,7 @@ class NamespacesCommand(Command):
"""
Convert a management API to a catalog API
"""
- catalog_host = re.match(r'(http://[^/]+)', api.api_client.configuration.host).group(1)
+ catalog_host = re.match(r'(https?://.+)/api/management', api.api_client.configuration.host).group(1)
configuration = Configuration(
host=f'{catalog_host}/api/catalog',
username=api.api_client.configuration.username,
@@ -71,12 +71,12 @@ class NamespacesCommand(Command):
def execute(self, api: PolarisDefaultApi) -> None:
catalog_api = self._get_catalog_api(api)
if self.namespaces_subcommand == Subcommands.CREATE:
- properties = self.properties or {}
+ req_properties = self.properties or {}
if self.location:
- properties = {**properties, Arguments.LOCATION: self.location}
+ req_properties = {**req_properties, Arguments.LOCATION: self.location}
request = CreateNamespaceRequest(
namespace=self.namespace,
- properties=self.properties
+ properties=req_properties
)
catalog_api.create_namespace(
prefix=self.catalog,
@@ -91,7 +91,8 @@ class NamespacesCommand(Command):
elif self.namespaces_subcommand == Subcommands.DELETE:
catalog_api.drop_namespace(prefix=self.catalog, namespace=UNIT_SEPARATOR.join(self.namespace))
elif self.namespaces_subcommand == Subcommands.GET:
- catalog_api.namespace_exists(prefix=self.catalog, namespace=UNIT_SEPARATOR.join(self.namespace))
- print(json.dumps({"namespace": '.'.join(self.namespace)}))
+ print(catalog_api.load_namespace_metadata(
+ prefix=self.catalog,
+ namespace=UNIT_SEPARATOR.join(self.namespace)).to_json())
else:
raise Exception(f"{self.namespaces_subcommand} is not supported in the CLI")
diff --git a/regtests/client/python/cli/command/principal_roles.py b/regtests/client/python/cli/command/principal_roles.py
index 4b7c0cfe..c64e363f 100644
--- a/regtests/client/python/cli/command/principal_roles.py
+++ b/regtests/client/python/cli/command/principal_roles.py
@@ -17,7 +17,7 @@
# under the License.
#
from dataclasses import dataclass
-from typing import Dict, Optional
+from typing import Dict, Optional, List
from pydantic import StrictStr
@@ -46,6 +46,8 @@ class PrincipalRolesCommand(Command):
catalog_name: str
catalog_role_name: str
properties: Optional[Dict[str, StrictStr]]
+ set_properties: Dict[str, StrictStr]
+ remove_properties: List[str]
def validate(self):
if self.principal_roles_subcommand == Subcommands.LIST:
@@ -82,9 +84,20 @@ class PrincipalRolesCommand(Command):
print(principal_role.to_json())
elif self.principal_roles_subcommand == Subcommands.UPDATE:
principal_role = api.get_principal_role(self.principal_role_name)
+ new_properties = principal_role.properties or {}
+
+ # Add or update all entries specified in set_properties
+ if self.set_properties:
+ new_properties = {**new_properties, **self.set_properties}
+
+ # Remove all keys specified in remove_properties
+ if self.remove_properties:
+ for to_remove in self.remove_properties:
+ new_properties.pop(to_remove, None)
+
request = UpdatePrincipalRoleRequest(
current_entity_version=principal_role.entity_version,
- properties=self.properties
+ properties=new_properties
)
api.update_principal_role(self.principal_role_name, request)
elif self.principal_roles_subcommand == Subcommands.GRANT:
diff --git a/regtests/client/python/cli/command/principals.py b/regtests/client/python/cli/command/principals.py
index e25de739..2de58fde 100644
--- a/regtests/client/python/cli/command/principals.py
+++ b/regtests/client/python/cli/command/principals.py
@@ -17,7 +17,7 @@
# under the License.
#
from dataclasses import dataclass
-from typing import Dict, Optional
+from typing import Dict, Optional, List
from pydantic import StrictStr
@@ -45,6 +45,8 @@ class PrincipalsCommand(Command):
client_id: str
principal_role: str
properties: Optional[Dict[str, StrictStr]]
+ set_properties: Dict[str, StrictStr]
+ remove_properties: List[str]
def validate(self):
pass
@@ -75,9 +77,20 @@ class PrincipalsCommand(Command):
print(api.rotate_credentials(self.principal_name).to_json())
elif self.principals_subcommand == Subcommands.UPDATE:
principal = api.get_principal(self.principal_name)
+ new_properties = principal.properties or {}
+
+ # Add or update all entries specified in set_properties
+ if self.set_properties:
+ new_properties = {**new_properties, **self.set_properties}
+
+ # Remove all keys specified in remove_properties
+ if self.remove_properties:
+ for to_remove in self.remove_properties:
+ new_properties.pop(to_remove, None)
+
request = UpdatePrincipalRequest(
- current_entity_version=principal.current_entity_version,
- properties=self.properties
+ current_entity_version=principal.entity_version,
+ properties=new_properties
)
api.update_principal(self.principal_name, request)
else:
diff --git a/regtests/client/python/cli/constants.py b/regtests/client/python/cli/constants.py
index 9831bfb5..42bcbf96 100644
--- a/regtests/client/python/cli/constants.py
+++ b/regtests/client/python/cli/constants.py
@@ -117,6 +117,8 @@ class Arguments:
CLIENT_ID = 'client_id'
PRINCIPAL_ROLE = 'principal_role'
PROPERTY = 'property'
+ SET_PROPERTY = 'set_property'
+ REMOVE_PROPERTY = 'remove_property'
PRIVILEGE = 'privilege'
NAMESPACE = 'namespace'
TABLE = 'table'
@@ -126,6 +128,7 @@ class Arguments:
ACCESS_TOKEN = 'access_token'
HOST = 'host'
PORT = 'port'
+ BASE_URL = 'base_url'
PARENT = 'parent'
LOCATION = 'location'
@@ -139,6 +142,13 @@ class Hints:
PROPERTY = ('A key/value pair such as: tag=value. Multiple can be provided by specifying this option'
' more than once')
+ SET_PROPERTY = ('A key/value pair such as: tag=value. Merges the specified key/value into an existing'
+ ' properties map by updating the value if the key already exists or creating a new'
+ ' entry if not. Multiple can be provided by specifying this option more than once')
+ REMOVE_PROPERTY = ('A key to remove from a properties map. If the key already does not exist then'
+ ' no action is takn for the specified key. If properties are also being set in'
+ ' the same update command then the list of removals is applied last. Multiple'
+ ' can be provided by specifying this option more than once')
class Catalogs:
GRANT = 'Grant a catalog role to a catalog'
@@ -217,3 +227,5 @@ class Hints:
UNIT_SEPARATOR = chr(0x1F)
CLIENT_ID_ENV = 'CLIENT_ID'
CLIENT_SECRET_ENV = 'CLIENT_SECRET'
+DEFAULT_HOSTNAME = 'localhost'
+DEFAULT_PORT = 8181
diff --git a/regtests/client/python/cli/options/option_tree.py b/regtests/client/python/cli/options/option_tree.py
index 00f54037..f19d2340 100644
--- a/regtests/client/python/cli/options/option_tree.py
+++ b/regtests/client/python/cli/options/option_tree.py
@@ -104,7 +104,8 @@ class OptionTree:
Argument(Arguments.DEFAULT_BASE_LOCATION, str, Hints.Catalogs.Update.DEFAULT_BASE_LOCATION),
Argument(Arguments.ALLOWED_LOCATION, str, Hints.Catalogs.Create.ALLOWED_LOCATION,
allow_repeats=True),
- Argument(Arguments.PROPERTY, str, Hints.PROPERTY, allow_repeats=True),
+ Argument(Arguments.SET_PROPERTY, str, Hints.SET_PROPERTY, allow_repeats=True),
+ Argument(Arguments.REMOVE_PROPERTY, str, Hints.REMOVE_PROPERTY, allow_repeats=True),
], input_name=Arguments.CATALOG)
]),
Option(Commands.PRINCIPALS, 'manage principals', children=[
@@ -118,7 +119,8 @@ class OptionTree:
Option(Subcommands.LIST),
Option(Subcommands.ROTATE_CREDENTIALS, input_name=Arguments.PRINCIPAL),
Option(Subcommands.UPDATE, args=[
- Argument(Arguments.PROPERTY, str, Hints.PROPERTY, allow_repeats=True)
+ Argument(Arguments.SET_PROPERTY, str, Hints.SET_PROPERTY, allow_repeats=True),
+ Argument(Arguments.REMOVE_PROPERTY, str, Hints.REMOVE_PROPERTY, allow_repeats=True),
], input_name=Arguments.PRINCIPAL)
]),
Option(Commands.PRINCIPAL_ROLES, 'manage principal roles', children=[
@@ -132,7 +134,8 @@ class OptionTree:
Argument(Arguments.PRINCIPAL, str, Hints.PrincipalRoles.List.PRINCIPAL_NAME)
]),
Option(Subcommands.UPDATE, args=[
- Argument(Arguments.PROPERTY, str, Hints.PROPERTY, allow_repeats=True)
+ Argument(Arguments.SET_PROPERTY, str, Hints.SET_PROPERTY, allow_repeats=True),
+ Argument(Arguments.REMOVE_PROPERTY, str, Hints.REMOVE_PROPERTY, allow_repeats=True),
], input_name=Arguments.PRINCIPAL_ROLE),
Option(Subcommands.GRANT, hint=Hints.PrincipalRoles.GRANT, args=[
Argument(Arguments.PRINCIPAL, str, Hints.PrincipalRoles.Grant.PRINCIPAL)
@@ -157,7 +160,8 @@ class OptionTree:
], input_name=Arguments.CATALOG),
Option(Subcommands.UPDATE, args=[
Argument(Arguments.CATALOG, str, Hints.CatalogRoles.CATALOG_NAME),
- Argument(Arguments.PROPERTY, str, Hints.PROPERTY, allow_repeats=True)
+ Argument(Arguments.SET_PROPERTY, str, Hints.SET_PROPERTY, allow_repeats=True),
+ Argument(Arguments.REMOVE_PROPERTY, str, Hints.REMOVE_PROPERTY, allow_repeats=True),
], input_name=Arguments.CATALOG_ROLE),
Option(Subcommands.GRANT, hint=Hints.CatalogRoles.GRANT_CATALOG_ROLE, args=[
Argument(Arguments.CATALOG, str, Hints.CatalogRoles.CATALOG_NAME),
diff --git a/regtests/client/python/cli/options/parser.py b/regtests/client/python/cli/options/parser.py
index d014a1d8..e4a6a6b3 100644
--- a/regtests/client/python/cli/options/parser.py
+++ b/regtests/client/python/cli/options/parser.py
@@ -37,8 +37,9 @@ class Parser(object):
"""
_ROOT_ARGUMENTS = [
- Argument(Arguments.HOST, str, hint='hostname', default='localhost'),
- Argument(Arguments.PORT, int, hint='port', default=8181),
+ Argument(Arguments.HOST, str, hint='hostname'),
+ Argument(Arguments.PORT, int, hint='port'),
+ Argument(Arguments.BASE_URL, str, hint='complete base URL instead of hostname:port'),
Argument(Arguments.CLIENT_ID, str, hint='client ID for token-based authentication'),
Argument(Arguments.CLIENT_SECRET, str, hint='client secret for token-based authentication'),
Argument(Arguments.ACCESS_TOKEN, str, hint='access token for token-based authentication'),
diff --git a/regtests/client/python/cli/polaris_cli.py b/regtests/client/python/cli/polaris_cli.py
index 397b192e..9dd98127 100644
--- a/regtests/client/python/cli/polaris_cli.py
+++ b/regtests/client/python/cli/polaris_cli.py
@@ -22,13 +22,12 @@ import os
import sys
from json import JSONDecodeError
-from cli.constants import Arguments, CLIENT_ID_ENV, CLIENT_SECRET_ENV
+from cli.constants import Arguments, CLIENT_ID_ENV, CLIENT_SECRET_ENV, DEFAULT_HOSTNAME, DEFAULT_PORT
from cli.options.option_tree import Argument
from cli.options.parser import Parser
from polaris.management import ApiClient, Configuration
from polaris.management import PolarisDefaultApi
-
class PolarisCli:
"""
Implements a basic Command-Line Interface (CLI) for interacting with a Polaris service. The CLI can be used to
@@ -36,9 +35,10 @@ class PolarisCli:
available in the Python client API.
Example usage:
- * ./polaris --client-id ${id} --client-secret ${secret} --host ${hostname} principals create example_user
- * ./polaris --client-id ${id} --client-secret ${secret} --host ${hostname} principal-roles create example_role
- * ./polaris --client-id ${id} --client-secret ${secret} --host ${hostname} catalog-roles list
+ * ./polaris --client-id ${id} --client-secret ${secret} --host ${hostname} --port ${port} principals create example_user
+ * ./polaris --client-id ${id} --client-secret ${secret} --host ${hostname} --port ${port} principal-roles create example_role
+ * ./polaris --client-id ${id} --client-secret ${secret} --host ${hostname} --port ${port} catalog-roles list
+ * ./polaris --client-id ${id} --client-secret ${secret} --base-url https://custom-polaris-domain.example.com/service-prefix catalogs list
"""
# Can be enabled if the client is able to authenticate directly without first fetching a token
@@ -108,8 +108,20 @@ class PolarisCli:
f' Alternatively, you may set the environment variables {CLIENT_ID_ENV} &'
f' {CLIENT_SECRET_ENV}.')
# Authenticate accordingly
- polaris_management_url = f'http://{options.host}:{options.port}/api/management/v1'
- polaris_catalog_url = f'http://{options.host}:{options.port}/api/catalog/v1'
+ if options.base_url:
+ if options.host is not None or options.port is not None:
+ raise Exception(f'Please provide either {Argument.to_flag_name(Arguments.BASE_URL)} or'
+ f' {Argument.to_flag_name(Arguments.HOST)} &'
+ f' {Argument.to_flag_name(Arguments.PORT)}, but not both');
+
+ polaris_management_url = f'{options.base_url}/api/management/v1'
+ polaris_catalog_url = f'{options.base_url}/api/catalog/v1'
+ else:
+ host = options.host or DEFAULT_HOSTNAME
+ port = options.port or DEFAULT_PORT
+ polaris_management_url = f'http://{host}:{port}/api/management/v1'
+ polaris_catalog_url = f'http://{host}:{port}/api/catalog/v1'
+
builder = None
if has_access_token:
builder = lambda: ApiClient(
diff --git a/regtests/client/python/test/test_cli_parsing.py b/regtests/client/python/test/test_cli_parsing.py
index 32a96b12..27c6b9e1 100644
--- a/regtests/client/python/test/test_cli_parsing.py
+++ b/regtests/client/python/test/test_cli_parsing.py
@@ -50,10 +50,18 @@ class TestCliParsing(unittest.TestCase):
Parser.parse(['catalogs', 'create', 'catalog_name', '--type', 'BANANA']) # invalid catalog type
self.assertEqual(cm.exception.code, INVALID_ARGS)
+ with self.assertRaises(SystemExit) as cm:
+ Parser.parse(['catalogs', 'create', 'catalog_name', '--set-property', 'foo=bar']) # can't use --set-property on create
+ self.assertEqual(cm.exception.code, INVALID_ARGS)
+
with self.assertRaises(SystemExit) as cm:
Parser.parse(['catalogs', 'get', 'catalog_name', '--fake-flag'])
self.assertEqual(cm.exception.code, INVALID_ARGS)
+ with self.assertRaises(SystemExit) as cm:
+ Parser.parse(['catalogs', 'update', 'catalog_name', '--property', 'foo=bar'])
+ self.assertEqual(cm.exception.code, INVALID_ARGS)
+
with self.assertRaises(SystemExit) as cm:
Parser.parse(['principals', 'create', 'name', '--type', 'bad'])
self.assertEqual(cm.exception.code, INVALID_ARGS)
@@ -105,6 +113,7 @@ class TestCliParsing(unittest.TestCase):
Parser.parse(['catalogs', 'get', 'catalog_name'])
Parser.parse(['principals', 'list'])
Parser.parse(['--host', 'some-host', 'catalogs', 'list'])
+ Parser.parse(['--base-url', 'https://customservice.com/subpath', 'catalogs', 'list'])
Parser.parse(['privileges', 'catalog', 'grant', '--catalog', 'foo', '--catalog-role', 'bar', 'TABLE_READ_DATA'])
Parser.parse(['privileges', 'table', 'grant', '--catalog', 'foo', '--catalog-role', 'bar',
'--namespace', 'n', '--table', 't', 'TABLE_READ_DATA'])
@@ -184,7 +193,7 @@ class TestCliParsing(unittest.TestCase):
'--default-base-location')
check_exception(lambda: mock_execute(['catalog-roles', 'get', 'foo']),
'--catalog')
- check_exception(lambda: mock_execute(['catalogs', 'update', 'foo', '--property', 'bad-format']),
+ check_exception(lambda: mock_execute(['catalogs', 'update', 'foo', '--set-property', 'bad-format']),
'bad-format')
check_exception(lambda: mock_execute(['privileges', 'catalog', 'grant',
'--catalog', 'foo', '--catalog-role', 'bar', 'TABLE_READ_MORE_BOOKS']),
@@ -234,6 +243,8 @@ class TestCliParsing(unittest.TestCase):
(0, 'catalog.storage_config_info.gcs_service_account'): 'sa',
})
check_arguments(mock_execute(['catalogs', 'list']), 'list_catalogs')
+ check_arguments(mock_execute([
+ '--base-url', 'https://customservice.com/subpath', 'catalogs', 'list']), 'list_catalogs')
check_arguments(mock_execute(['catalogs', 'delete', 'foo']), 'delete_catalog', {
(0, None): 'foo',
})
@@ -245,6 +256,21 @@ class TestCliParsing(unittest.TestCase):
'get_catalog', {
(0, None): 'foo',
})
+ check_arguments(
+ mock_execute(['catalogs', 'update', 'foo', '--set-property', 'key=value']),
+ 'get_catalog', {
+ (0, None): 'foo',
+ })
+ check_arguments(
+ mock_execute(['catalogs', 'update', 'foo', '--remove-property', 'key']),
+ 'get_catalog', {
+ (0, None): 'foo',
+ })
+ check_arguments(
+ mock_execute(['catalogs', 'update', 'foo', '--set-property', 'key=value', '--default-base-location', 'x']),
+ 'get_catalog', {
+ (0, None): 'foo',
+ })
check_arguments(
mock_execute(['principals', 'create', 'foo', '--property', 'key=value']),
'create_principal', {
@@ -269,7 +295,12 @@ class TestCliParsing(unittest.TestCase):
(0, None): 'foo',
})
check_arguments(
- mock_execute(['principals', 'update', 'foo', '--property', 'key=value']),
+ mock_execute(['principals', 'update', 'foo', '--set-property', 'key=value']),
+ 'get_principal', {
+ (0, None): 'foo',
+ })
+ check_arguments(
+ mock_execute(['principals', 'update', 'foo', '--remove-property', 'key']),
'get_principal', {
(0, None): 'foo',
})
@@ -305,12 +336,12 @@ class TestCliParsing(unittest.TestCase):
(0, None): 'foo',
})
check_arguments(
- mock_execute(['principal-roles', 'update', 'foo', '--property', 'key=value']),
+ mock_execute(['principal-roles', 'update', 'foo', '--set-property', 'key=value']),
'get_principal_role', {
(0, None): 'foo'
})
check_arguments(
- mock_execute(['principal-roles', 'update', 'foo', '--property', 'key=value']),
+ mock_execute(['principal-roles', 'update', 'foo', '--remove-property', 'key']),
'get_principal_role', {
(0, None): 'foo',
})
@@ -360,7 +391,13 @@ class TestCliParsing(unittest.TestCase):
(1, None): 'foo',
})
check_arguments(mock_execute(
- ['catalog-roles', 'update', 'foo', '--catalog', 'bar', '--property', 'key=value']),
+ ['catalog-roles', 'update', 'foo', '--catalog', 'bar', '--set-property', 'key=value']),
+ 'get_catalog_role', {
+ (0, None): 'bar',
+ (1, None): 'foo',
+ })
+ check_arguments(mock_execute(
+ ['catalog-roles', 'update', 'foo', '--catalog', 'bar', '--remove-property', 'key']),
'get_catalog_role', {
(0, None): 'bar',
(1, None): 'foo',
diff --git a/regtests/t_cli/src/test_cli.py b/regtests/t_cli/src/test_cli.py
index 2a058aa4..4395caf1 100644
--- a/regtests/t_cli/src/test_cli.py
+++ b/regtests/t_cli/src/test_cli.py
@@ -29,7 +29,8 @@ from typing import Callable
CLI_PYTHONPATH = f'{os.path.dirname(os.path.abspath(__file__))}/../../client/python'
ROLE_ARN = 'arn:aws:iam::123456789012:role/my-role'
-POLARIS_URL = 'http://polaris:8181/api/catalog/v1/oauth/tokens'
+POLARIS_HOST = os.getenv('POLARIS_HOST') or 'polaris'
+POLARIS_URL = f'http://{POLARIS_HOST}:8181/api/catalog/v1/oauth/tokens'
def get_salt(length=8) -> str:
characters = string.ascii_letters + string.digits
@@ -49,7 +50,7 @@ def cli(access_token):
'--access-token',
access_token,
'--host',
- 'polaris',
+ POLARIS_HOST,
*args],
capture_output=True,
text=True
@@ -167,10 +168,21 @@ def test_quickstart_flow():
'create',
'--catalog',
f'test_cli_catalog_{SALT}',
- f'test_cli_namespace_{SALT}'
+ f'test_cli_namespace_{SALT}',
+ '--property',
+ 'foo=bar',
+ '--location',
+ 's3://custom-namespace-location'
), checker=lambda s: s == '')
check_output(cli(user_token)('namespaces', 'list', '--catalog', f'test_cli_catalog_{SALT}'),
checker=lambda s: f'test_cli_namespace_{SALT}' in s)
+ check_output(cli(user_token)(
+ 'namespaces',
+ 'get',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ f'test_cli_namespace_{SALT}'
+ ), checker=lambda s: 's3://custom-namespace-location' in s and '"foo": "bar"' in s)
check_output(cli(user_token)(
'namespaces',
'delete',
@@ -185,6 +197,369 @@ def test_quickstart_flow():
sys.path.pop(0)
pass
+def test_update_catalog():
+ """
+ Test updating properties on a catalog
+ """
+ SALT = get_salt()
+ sys.path.insert(0, CLI_PYTHONPATH)
+ try:
+
+ # Create a catalog:
+ check_output(root_cli(
+ 'catalogs',
+ 'create',
+ '--storage-type',
+ 's3',
+ '--role-arn',
+ ROLE_ARN,
+ '--default-base-location',
+ f's3://fake-location-{SALT}',
+ f'test_cli_catalog_{SALT}'), checker=lambda s: s == '')
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: 's3://fake-location' in s)
+
+ # Update by adding a property
+ check_output(root_cli(
+ 'catalogs',
+ 'update',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'foo=bar'
+ ), checker=lambda s: s == '')
+
+ # Make sure the base location didn't get clobbered, and the new property is present.
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: 's3://fake-location' in s and '"foo": "bar"' in s)
+
+ # Update by changing default-base-location
+ check_output(root_cli(
+ 'catalogs',
+ 'update',
+ f'test_cli_catalog_{SALT}',
+ '--default-base-location',
+ f's3://new-location-{SALT}'
+ ), checker=lambda s: s == '')
+
+ # Check for new location, and make sure the custom property is still there.
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: 's3://new-location' in s and '"foo": "bar"' in s)
+
+ # Update by adding a second property
+ check_output(root_cli(
+ 'catalogs',
+ 'update',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'prop2=222'
+ ), checker=lambda s: s == '')
+
+ # Make sure both custom properties are present even though we only added the second one
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: '"prop2": "222"' in s and '"foo": "bar"' in s)
+
+ # Update by modifying a property and adding a property at the same time
+ check_output(root_cli(
+ 'catalogs',
+ 'update',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'prop2=two',
+ '--set-property',
+ 'prop3=333'
+ ), checker=lambda s: s == '')
+
+ # Check all three custom properties
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: '"prop2": "two"' in s and '"foo": "bar"' in s and '"prop3": "333"' in s)
+
+ # Update one property that will be removed in the same command, one that will not be removed,
+ # add one new property, remove one of the properties that is not updated, and remove a
+ # nonexistent property
+ check_output(root_cli(
+ 'catalogs',
+ 'update',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'prop2=2222',
+ '--set-property',
+ 'prop3=3333',
+ '--set-property',
+ 'prop4=4444',
+ '--remove-property',
+ 'foo',
+ '--remove-property',
+ 'prop2',
+ '--remove-property',
+ 'nonexistent'
+ ), checker=lambda s: s == '')
+
+ # One updated property and one new property remain after removals
+ check_output(root_cli('catalogs', 'get', f'test_cli_catalog_{SALT}'),
+ checker=lambda s: 'foo' not in s and 'prop2' not in s and
+ '"prop3": "3333"' in s and '"prop4": "4444"' in s)
+
+ finally:
+ sys.path.pop(0)
+ pass
+
+def test_update_principal():
+ """
+ Test updating properties on a principal
+ """
+ SALT = get_salt()
+ sys.path.insert(0, CLI_PYTHONPATH)
+ try:
+
+ # Create a principal:
+ credentials = root_cli('principals', 'create', f'test_cli_user_{SALT}', '--property', 'foo=bar')()
+ check_output(root_cli('principals', 'get', f'test_cli_user_{SALT}'),
+ checker=lambda s: '"foo": "bar"' in s)
+
+ # Update by adding a second property
+ check_output(root_cli(
+ 'principals',
+ 'update',
+ f'test_cli_user_{SALT}',
+ '--set-property',
+ 'prop2=222'
+ ), checker=lambda s: s == '')
+
+ # Make sure both custom properties are present even though we only added the second one
+ check_output(root_cli('principals', 'get', f'test_cli_user_{SALT}'),
+ checker=lambda s: '"prop2": "222"' in s and '"foo": "bar"' in s)
+
+ # Update by modifying a property and adding a property at the same time
+ check_output(root_cli(
+ 'principals',
+ 'update',
+ f'test_cli_user_{SALT}',
+ '--set-property',
+ 'prop2=two',
+ '--set-property',
+ 'prop3=333'
+ ), checker=lambda s: s == '')
+
+ # Check all three custom properties
+ check_output(root_cli('principals', 'get', f'test_cli_user_{SALT}'),
+ checker=lambda s: '"prop2": "two"' in s and '"foo": "bar"' in s and '"prop3": "333"' in s)
+
+ # Update one property that will be removed in the same command, one that will not be removed,
+ # add one new property, remove one of the properties that is not updated, and remove a
+ # nonexistent property
+ check_output(root_cli(
+ 'principals',
+ 'update',
+ f'test_cli_user_{SALT}',
+ '--set-property',
+ 'prop2=2222',
+ '--set-property',
+ 'prop3=3333',
+ '--set-property',
+ 'prop4=4444',
+ '--remove-property',
+ 'foo',
+ '--remove-property',
+ 'prop2',
+ '--remove-property',
+ 'nonexistent'
+ ), checker=lambda s: s == '')
+
+ # One updated property and one new property remain after removals
+ check_output(root_cli('principals', 'get', f'test_cli_user_{SALT}'),
+ checker=lambda s: 'foo' not in s and 'prop2' not in s and
+ '"prop3": "3333"' in s and '"prop4": "4444"' in s)
+
+ finally:
+ sys.path.pop(0)
+ pass
+
+def test_update_principal_role():
+ """
+ Test updating properties on a principal_role
+ """
+ SALT = get_salt()
+ sys.path.insert(0, CLI_PYTHONPATH)
+ try:
+
+ # Create a principal_role:
+ credentials = root_cli('principal-roles', 'create', f'test_cli_p_role_{SALT}', '--property', 'foo=bar')()
+ check_output(root_cli('principal-roles', 'get', f'test_cli_p_role_{SALT}'),
+ checker=lambda s: '"foo": "bar"' in s)
+
+ # Update by adding a second property
+ check_output(root_cli(
+ 'principal-roles',
+ 'update',
+ f'test_cli_p_role_{SALT}',
+ '--set-property',
+ 'prop2=222'
+ ), checker=lambda s: s == '')
+
+ # Make sure both custom properties are present even though we only added the second one
+ check_output(root_cli('principal-roles', 'get', f'test_cli_p_role_{SALT}'),
+ checker=lambda s: '"prop2": "222"' in s and '"foo": "bar"' in s)
+
+ # Update by modifying a property and adding a property at the same time
+ check_output(root_cli(
+ 'principal-roles',
+ 'update',
+ f'test_cli_p_role_{SALT}',
+ '--set-property',
+ 'prop2=two',
+ '--set-property',
+ 'prop3=333'
+ ), checker=lambda s: s == '')
+
+ # Check all three custom properties
+ check_output(root_cli('principal-roles', 'get', f'test_cli_p_role_{SALT}'),
+ checker=lambda s: '"prop2": "two"' in s and '"foo": "bar"' in s and '"prop3": "333"' in s)
+
+ # Update one property that will be removed in the same command, one that will not be removed,
+ # add one new property, remove one of the properties that is not updated, and remove a
+ # nonexistent property
+ check_output(root_cli(
+ 'principal-roles',
+ 'update',
+ f'test_cli_p_role_{SALT}',
+ '--set-property',
+ 'prop2=2222',
+ '--set-property',
+ 'prop3=3333',
+ '--set-property',
+ 'prop4=4444',
+ '--remove-property',
+ 'foo',
+ '--remove-property',
+ 'prop2',
+ '--remove-property',
+ 'nonexistent'
+ ), checker=lambda s: s == '')
+
+ # One updated property and one new property remain after removals
+ check_output(root_cli('principal-roles', 'get', f'test_cli_p_role_{SALT}'),
+ checker=lambda s: 'foo' not in s and 'prop2' not in s and
+ '"prop3": "3333"' in s and '"prop4": "4444"' in s)
+
+ finally:
+ sys.path.pop(0)
+ pass
+
+def test_update_catalog_role():
+ """
+ Test updating properties on a catalog_role
+ """
+ SALT = get_salt()
+ sys.path.insert(0, CLI_PYTHONPATH)
+ try:
+
+ # Create a catalog:
+ check_output(root_cli(
+ 'catalogs',
+ 'create',
+ '--storage-type',
+ 's3',
+ '--role-arn',
+ ROLE_ARN,
+ '--default-base-location',
+ f's3://fake-location-{SALT}',
+ f'test_cli_catalog_{SALT}'), checker=lambda s: s == '')
+
+ # Create a catalog_role:
+ credentials = root_cli(
+ 'catalog-roles',
+ 'create',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ '--property',
+ 'foo=bar')()
+ check_output(root_cli(
+ 'catalog-roles',
+ 'get',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ ), checker=lambda s: '"foo": "bar"' in s)
+
+ # Update by adding a second property
+ check_output(root_cli(
+ 'catalog-roles',
+ 'update',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'prop2=222'
+ ), checker=lambda s: s == '')
+
+ # Make sure both custom properties are present even though we only added the second one
+ check_output(root_cli(
+ 'catalog-roles',
+ 'get',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ ), checker=lambda s: '"prop2": "222"' in s and '"foo": "bar"' in s)
+
+ # Update by modifying a property and adding a property at the same time
+ check_output(root_cli(
+ 'catalog-roles',
+ 'update',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'prop2=two',
+ '--set-property',
+ 'prop3=333'
+ ), checker=lambda s: s == '')
+
+ # Check all three custom properties
+ check_output(root_cli(
+ 'catalog-roles',
+ 'get',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ ), checker=lambda s: '"prop2": "two"' in s and '"foo": "bar"' in s and '"prop3": "333"' in s)
+
+ # Update one property that will be removed in the same command, one that will not be removed,
+ # add one new property, remove one of the properties that is not updated, and remove a
+ # nonexistent property
+ check_output(root_cli(
+ 'catalog-roles',
+ 'update',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ '--set-property',
+ 'prop2=2222',
+ '--set-property',
+ 'prop3=3333',
+ '--set-property',
+ 'prop4=4444',
+ '--remove-property',
+ 'foo',
+ '--remove-property',
+ 'prop2',
+ '--remove-property',
+ 'nonexistent'
+ ), checker=lambda s: s == '')
+
+ # One updated property and one new property remain after removals
+ check_output(root_cli(
+ 'catalog-roles',
+ 'get',
+ f'test_cli_c_role_{SALT}',
+ '--catalog',
+ f'test_cli_catalog_{SALT}',
+ ), checker=lambda s: 'foo' not in s and 'prop2' not in s and
+ '"prop3": "3333"' in s and '"prop4": "4444"' in s)
+
+ finally:
+ sys.path.pop(0)
+ pass
def test_nested_namespace():
"""
@@ -253,7 +628,7 @@ def test_nested_namespace():
'--catalog',
f'test_cli_catalog_{SALT}',
f'a_{SALT}.b_{SALT}.c_{SALT}'
- ), checker=lambda s: f'a_{SALT}.b_{SALT}.c_{SALT}' in s)
+ ), checker=lambda s: f'"a_{SALT}", "b_{SALT}", "c_{SALT}"' in s)
check_exception(root_cli(
'namespaces',
'delete',