mchades commented on code in PR #14623:
URL: https://github.com/apache/gravitino/pull/14623#discussion_r4467353370
##########
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:
Reproduced on `0d0828c` using the real `HTTPClient` against a local HTTP
stub: a successful `DELETE` returning status 204 and an empty body causes
`remove_policy_from_tag()` to raise `JSONDecodeError: Expecting value: line 1
column 1 (char 0)`. The new test uses
`mock_http_response(BaseResponse(0).to_json())`, which supplies HTTP 200 with
JSON, so it misses the server's actual success response.
##########
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:
Reproduced on `0d0828c` through the real `HTTPClient` with local HTTP error
responses: HTTP 404 / code 1003 / type `NoSuchPolicyException` becomes
`NotFoundException` for both add and remove; HTTP 409 / code 1004 / type
`PolicyAlreadyAssociatedException` becomes `AlreadyExistsException` for add.
Catching the documented policy-specific exceptions therefore fails. Please
cover these response types in the regression tests.
##########
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:
Confirmed on `0d0828c`: `catalog.as_schemas().load_schema("s")` returns a
`GenericSchema` with neither `supports_policies()` nor `list_policies()`, and
it is not a `SupportsPolicies`. There are no production references to this
helper outside its own file. The new tests instantiate the helper directly, so
they pass without a usable entry point on loaded objects; please exercise
policy lookup through a loaded schema or table.
--
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]
