dennishuo commented on code in PR #988:
URL: https://github.com/apache/polaris/pull/988#discussion_r1951965854
##########
regtests/client/python/cli/command/catalogs.py:
##########
@@ -199,12 +194,33 @@ def execute(self, api: PolarisDefaultApi) -> None:
default_base_location=new_default_base_location,
additional_properties=new_additional_properties
)
- 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):
+
+ if (self._has_aws_storage_info() or self._has_azure_storage_info()
or
+ self._has_gcs_storage_info() or self.allowed_locations):
+ # We must first reconstitute local storage-config related
settings from the existing
+ # catalog to properly construct the complete updated
storage-config
+ updated_storage_info = catalog.storage_config_info
+
+ # In order to apply mutations client-side, we can't just use
the base
+ # _build_storage_config_info helper; instead, each allowed
updatable field defined
+ # in option_tree.py should be applied individually against the
existing
+ # storage_config_info here.
+ if (self.allowed_locations):
+
updated_storage_info.allowed_locations.extend(self.allowed_locations)
Review Comment:
Yeah, I considered override, but because of the way the flag is parsed as a
"repetition" of `--allowed_location`, e.g.:
catalogs create --allowed_location loc1 --allowed_location loc2
catalogs update --allowed_location loc3
with the semantic that `--allowed_location` is at least additive within a
single command, it seemed safer to "extend" for now.
Also, it seems like the most common scenario will be additive, unless tables
get unregistered carefully. For removals we might want smarter server-side
validation that existing locations aren't violated before we make it too easy
to remove items from this config.
Override I agree is more multi-purpose but I'm worried about people
accidentally breaking their prod config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
