Copilot commented on code in PR #14623:
URL: https://github.com/apache/gravitino/pull/14623#discussion_r4467155445
##########
clients/client-python/gravitino/client/gravitino_metalake.py:
##########
@@ -846,6 +879,377 @@ def delete_tag(self, tag_name) -> bool:
return drop_response.dropped()
+ def list_policy_associations_for_tag(self, tag_name) ->
list[PolicyTagAssociation]:
+ """
+ List the policy associations of a tag under a metalake.
+
+ Args:
+ tag_name (str): The name of the tag.
+
+ Returns:
+ list[PolicyTagAssociation]: The policy associations of the tag.
+
+ Raises:
+ NoSuchTagException: If the tag does not exist.
+ """
+ url = self.API_METALAKES_TAG_POLICIES_PATH.format(
+ encode_string(self.name()), encode_string(tag_name)
+ )
+ response = self.rest_client.get(
+ url,
+ params={"details": "true"},
+ error_handler=TAG_ERROR_HANDLER,
+ )
+ resp = PolicyForTagAssociationListResponse.from_json(
+ response.body, infer_missing=True
+ )
+ resp.validate()
+
+ tag = self.get_tag(tag_name)
+ return [
+ GenericPolicyTagAssociation(
+ GenericPolicy(association.policy()),
+ tag,
+ association.selector().to_selector(),
+ )
+ for association in resp.associations()
+ ]
+
+ def add_policy_for_tag(
+ self,
+ tag_name,
+ policy_name,
+ selector: PolicyAssociationSelector = None,
+ ) -> PolicyTagAssociation:
+ """
+ Associate a policy with a tag under a metalake. When no selector is
+ given, the association applies to all values of the tag.
+
+ Args:
+ tag_name (str): The name of the tag.
+ policy_name (str): The name of the policy.
+ selector (PolicyAssociationSelector): The selector scoping the
+ association, or None for all tag values.
+
+ Returns:
+ PolicyTagAssociation: The created policy association.
+
+ Raises:
+ NoSuchTagException: If the tag does not exist.
+ NoSuchPolicyException: If the policy does not exist.
+ PolicyAlreadyAssociatedException: If the policy is already
+ associated with the tag.
+ """
+ if selector is None:
+ selector = AllValuesSelector.get()
+ add_request = PolicyTagAddRequest(
+ PolicyAssociationSelectorDTO.from_selector(selector)
+ )
+ add_request.validate()
+
+ url = self.API_METALAKES_TAG_POLICY_PATH.format(
+ encode_string(self.name()),
+ encode_string(tag_name),
+ encode_string(policy_name),
+ )
+ response = self.rest_client.post(
+ url,
+ json=add_request,
+ error_handler=TAG_ERROR_HANDLER,
+ )
+ resp = PolicyTagAssociationResponse.from_json(response.body,
infer_missing=True)
+ resp.validate()
+
+ return GenericPolicyTagAssociation(
+ self.get_policy(resp.policy()),
+ self.get_tag(resp.tag()),
+ resp.selector().to_selector(),
+ )
+
+ def remove_policy_from_tag(self, tag_name, policy_name) -> None:
+ """
+ Remove the association between a policy and a tag under a metalake.
+
+ Args:
+ tag_name (str): The name of the tag.
+ policy_name (str): The name of the policy.
+
+ Raises:
+ NoSuchTagException: If the tag does not exist.
+ NoSuchPolicyException: If the policy does not exist.
+ """
+ url = self.API_METALAKES_TAG_POLICY_PATH.format(
+ encode_string(self.name()),
+ encode_string(tag_name),
+ encode_string(policy_name),
+ )
+ response = self.rest_client.delete(
+ url,
+ error_handler=TAG_ERROR_HANDLER,
+ )
+ resp = BaseResponse.from_json(response.body, infer_missing=True)
+ resp.validate()
Review Comment:
The server's `TagOperations.removePolicyFromTag` returns HTTP 204 with no
body on success. `HTTPClient` preserves that empty body, so parsing it here
raises `JSONDecodeError` after the association has already been removed. Ignore
the successful DELETE response body, and update the test to exercise a 204
response rather than the mocked JSON `BaseResponse`.
##########
clients/client-python/gravitino/client/gravitino_metalake.py:
##########
@@ -846,6 +879,377 @@ def delete_tag(self, tag_name) -> bool:
return drop_response.dropped()
+ def list_policy_associations_for_tag(self, tag_name) ->
list[PolicyTagAssociation]:
+ """
+ List the policy associations of a tag under a metalake.
+
+ Args:
+ tag_name (str): The name of the tag.
+
+ Returns:
+ list[PolicyTagAssociation]: The policy associations of the tag.
+
+ Raises:
+ NoSuchTagException: If the tag does not exist.
+ """
+ url = self.API_METALAKES_TAG_POLICIES_PATH.format(
+ encode_string(self.name()), encode_string(tag_name)
+ )
+ response = self.rest_client.get(
+ url,
+ params={"details": "true"},
+ error_handler=TAG_ERROR_HANDLER,
+ )
+ resp = PolicyForTagAssociationListResponse.from_json(
+ response.body, infer_missing=True
+ )
+ resp.validate()
+
+ tag = self.get_tag(tag_name)
+ return [
+ GenericPolicyTagAssociation(
+ GenericPolicy(association.policy()),
+ tag,
+ association.selector().to_selector(),
+ )
+ for association in resp.associations()
+ ]
+
+ def add_policy_for_tag(
+ self,
+ tag_name,
+ policy_name,
+ selector: PolicyAssociationSelector = None,
+ ) -> PolicyTagAssociation:
+ """
+ Associate a policy with a tag under a metalake. When no selector is
+ given, the association applies to all values of the tag.
+
+ Args:
+ tag_name (str): The name of the tag.
+ policy_name (str): The name of the policy.
+ selector (PolicyAssociationSelector): The selector scoping the
+ association, or None for all tag values.
+
+ Returns:
+ PolicyTagAssociation: The created policy association.
+
+ Raises:
+ NoSuchTagException: If the tag does not exist.
+ NoSuchPolicyException: If the policy does not exist.
+ PolicyAlreadyAssociatedException: If the policy is already
+ associated with the tag.
+ """
+ if selector is None:
+ selector = AllValuesSelector.get()
+ add_request = PolicyTagAddRequest(
+ PolicyAssociationSelectorDTO.from_selector(selector)
+ )
+ add_request.validate()
+
+ url = self.API_METALAKES_TAG_POLICY_PATH.format(
+ encode_string(self.name()),
+ encode_string(tag_name),
+ encode_string(policy_name),
+ )
+ response = self.rest_client.post(
+ url,
+ json=add_request,
+ error_handler=TAG_ERROR_HANDLER,
Review Comment:
`TAG_ERROR_HANDLER` does not map `PolicyAlreadyAssociatedException` or
`NoSuchPolicyException`, so tag-side policy requests raise generic exceptions
that callers catching the documented policy exceptions will miss. Extend the
tag handler with both policy mappings while preserving its tag mappings, and
test duplicate-association and missing-policy responses. The missing-policy
mapping also affects `remove_policy_from_tag`.
##########
clients/client-python/gravitino/client/metadata_object_policy_operations.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from gravitino.api.metadata_object import MetadataObject
+from gravitino.api.policy.policy import Policy
+from gravitino.api.policy.policy_list import PolicyList
+from gravitino.api.policy.supports_policies import SupportsPolicies
+from gravitino.client.generic_policy import GenericPolicy
+from gravitino.dto.responses.policy_response import (
+ MetadataObjectPolicyListResponse,
+ PolicyNameListResponse,
+)
+from gravitino.exceptions.handlers.policy_error_handler import
POLICY_ERROR_HANDLER
+from gravitino.rest.rest_utils import encode_string
+from gravitino.utils.http_client import HTTPClient
+
+
+class MetadataObjectPolicyOperations(SupportsPolicies):
Review Comment:
This helper has no production callers, and returned metadata objects such as
`GenericSchema`, `BaseSchemaCatalog`, and `RelationalTable` expose no policy
operations. Users therefore cannot list associated policies or their visibility
flags through loaded objects. Compose the helper into supported metadata-object
implementations, expose the corresponding API accessors following the tag
pattern, and test listing through a loaded object.
--
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]
