Copilot commented on code in PR #9483: URL: https://github.com/apache/gravitino/pull/9483#discussion_r2621828574
########## clients/client-python/gravitino/client/generic_tag.py: ########## @@ -0,0 +1,150 @@ +# 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 typing import Optional + +from gravitino.api.metadata_object import MetadataObject +from gravitino.api.tag.tag import Tag +from gravitino.dto.audit_dto import AuditDTO +from gravitino.dto.responses.metadata_object_list_response import ( + MetadataObjectListResponse, +) +from gravitino.dto.tag_dto import TagDTO +from gravitino.exceptions.handlers.error_handler import ErrorHandler +from gravitino.exceptions.handlers.error_handlers import ErrorHandlers +from gravitino.rest.rest_utils import encode_string +from gravitino.utils import HTTPClient +from gravitino.utils.http_client import Response + + +class GenericTag(Tag, Tag.AssociatedObjects): + """Represents a generic tag.""" + + API_LIST_OBJECTS_ENDPOINT = "api/metalakes/{}/tags/{}/objects" + + def __init__( + self, + metalake: str, + tag_dto: TagDTO, + client: HTTPClient, + ) -> None: + self._metalake = metalake + self._tag_dto = tag_dto + self._client = client + + def __eq__(self, other: object) -> bool: + if not isinstance(other, GenericTag): + return False + + return self._tag_dto == other._tag_dto + + def __hash__(self) -> int: + return hash(self._tag_dto) + + def name(self) -> str: + """Get the name of the tag. + + Returns: + str: The name of the tag. + """ + return self._tag_dto.name() + + def comment(self) -> str: + """Get the comment of the tag. + + Returns: + str: The comment of the tag. + """ + return self._tag_dto.comment() + + def properties(self) -> dict[str, str]: + """Get the properties of the tag. + + Returns: + Dict[str, str]: The properties of the tag. + """ + return self._tag_dto.properties() + + def inherited(self) -> Optional[bool]: + """Check if the tag is inherited from a parent object or not. + + If the tag is inherited, it will return `True`, if it is owned by the object itself, it will return `False`. + + **Note**. The return value is optional, only when the tag is associated with an object, and called from the + object, the return value will be present. Otherwise, it will be empty. + + Returns: + Optional[bool]: + True if the tag is inherited, false if it is owned by the object itself. Empty if the + tag is not associated with any object. + """ + return self._tag_dto.inherited() + + def audit_info(self) -> AuditDTO: + """ + Retrieve The audit information of the entity. + + Returns: + AuditDTO: The audit information of the entity. + """ + return self._tag_dto.audit_info() + + def associated_objects(self) -> Tag.AssociatedObjects: + """ + The associated objects of the tag. + + Returns: + AssociatedObjects: The associated objects of the tag. + """ + return self + + def objects(self) -> list[MetadataObject]: + """ + Retrieve The list of objects that are associated with this tag. + + Returns: + list[MetadataObject]: The list of objects that are associated with this tag. + """ + url = self.API_LIST_OBJECTS_ENDPOINT.format( + self._metalake, + encode_string(self.name()), + ) + + response = self.get_response(url, ErrorHandlers.tag_error_handler()) + objects_resp = MetadataObjectListResponse.from_json( + response.body, infer_missing=True + ) + objects_resp.validate() + + return objects_resp.metadata_objects() + + def get_response(self, url: str, error_handler: ErrorHandler) -> Response: + """ + Get the response from the server. for test convenience. Review Comment: Grammatical error in documentation. The phrase 'for test convenience' should be 'for testing convenience' or 'for test convenience purposes' to be grammatically correct. ```suggestion Get the response from the server, for testing convenience. ``` ########## clients/client-python/gravitino/client/gravitino_metalake.py: ########## @@ -548,8 +570,18 @@ def get_tag(self, tag_name) -> Tag: Raises: NoSuchTagException: If the tag does not exist. """ - # TODO implement get_tag - raise NotImplementedError() + Precondition.check_string_not_empty( + tag_name, "tag name must not be null or empty" + ) + url = self.API_METALAKES_TAGS_PATH.format(encode_string(self.name())) Review Comment: The URL construction for getting a specific tag is incorrect. The get_tag method should append the tag_name to the base URL. Currently, it's using the same URL as list_tags, which would return all tags instead of a specific one. The correct pattern should be: url = f"{self.API_METALAKES_TAGS_PATH.format(encode_string(self.name()))}/{encode_string(tag_name)}" ```suggestion url = f"{self.API_METALAKES_TAGS_PATH.format(encode_string(self.name()))}/{encode_string(tag_name)}" ``` ########## clients/client-python/gravitino/dto/requests/tag_create_request.py: ########## @@ -0,0 +1,46 @@ +# 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 dataclasses import dataclass, field +from typing import Optional + +from dataclasses_json import config, dataclass_json + +from gravitino.rest.rest_message import RESTRequest +from gravitino.utils.precondition import Precondition + + +@dataclass_json +@dataclass +class TagCreateRequest(RESTRequest): + """Represents a request to create a tag.""" + + _name: str = field(metadata=config(field_name="name")) + _comment: Optional[str] = field(metadata=config(field_name="comment")) Review Comment: Missing default value for optional field. The _comment field is marked as Optional but doesn't have a default value. This will cause a dataclass error since it comes before _properties which has a default. Add 'default=None' to the field definition. ```suggestion _comment: Optional[str] = field(default=None, metadata=config(field_name="comment")) ``` ########## clients/client-python/gravitino/exceptions/handlers/error_handlers.py: ########## @@ -0,0 +1,51 @@ +# 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 gravitino.exceptions.handlers.job_error_handler import ( + JOB_ERROR_HANDLER, +) +from gravitino.exceptions.handlers.tag_error_handler import ( + TAG_ERROR_HANDLER, + RestErrorHandler, +) Review Comment: Incorrect import statement. RestErrorHandler is being imported from tag_error_handler module, but it should be imported from rest_error_handler module. The correct import should be: from gravitino.exceptions.handlers.rest_error_handler import RestErrorHandler ```suggestion ) from gravitino.exceptions.handlers.rest_error_handler import RestErrorHandler ``` ########## clients/client-python/gravitino/dto/responses/metadata_object_list_response.py: ########## @@ -0,0 +1,59 @@ +# 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 dataclasses import dataclass, field + +from dataclasses_json import config + +from gravitino.dto.metadata_object_dto import MetadataObjectDTO +from gravitino.dto.responses.base_response import BaseResponse +from gravitino.utils.precondition import Precondition + + +@dataclass +class MetadataObjectListResponse(BaseResponse): + """Represents a response containing a list of metadata objects.""" + + _metadata_objects: list[MetadataObjectDTO] = field( Review Comment: Missing default value for list field. The _metadata_objects field should have a default_factory to avoid potential initialization issues. Add default_factory=list to the field definition. ```suggestion _metadata_objects: list[MetadataObjectDTO] = field( default_factory=list, ``` ########## clients/client-python/gravitino/client/generic_tag.py: ########## @@ -0,0 +1,150 @@ +# 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 typing import Optional + +from gravitino.api.metadata_object import MetadataObject +from gravitino.api.tag.tag import Tag +from gravitino.dto.audit_dto import AuditDTO +from gravitino.dto.responses.metadata_object_list_response import ( + MetadataObjectListResponse, +) +from gravitino.dto.tag_dto import TagDTO +from gravitino.exceptions.handlers.error_handler import ErrorHandler +from gravitino.exceptions.handlers.error_handlers import ErrorHandlers +from gravitino.rest.rest_utils import encode_string +from gravitino.utils import HTTPClient +from gravitino.utils.http_client import Response + + +class GenericTag(Tag, Tag.AssociatedObjects): + """Represents a generic tag.""" + + API_LIST_OBJECTS_ENDPOINT = "api/metalakes/{}/tags/{}/objects" + + def __init__( + self, + metalake: str, + tag_dto: TagDTO, + client: HTTPClient, + ) -> None: + self._metalake = metalake + self._tag_dto = tag_dto + self._client = client + + def __eq__(self, other: object) -> bool: + if not isinstance(other, GenericTag): + return False + + return self._tag_dto == other._tag_dto + + def __hash__(self) -> int: + return hash(self._tag_dto) + + def name(self) -> str: + """Get the name of the tag. + + Returns: + str: The name of the tag. + """ + return self._tag_dto.name() + + def comment(self) -> str: + """Get the comment of the tag. + + Returns: + str: The comment of the tag. + """ + return self._tag_dto.comment() + + def properties(self) -> dict[str, str]: + """Get the properties of the tag. + + Returns: + Dict[str, str]: The properties of the tag. + """ + return self._tag_dto.properties() + + def inherited(self) -> Optional[bool]: + """Check if the tag is inherited from a parent object or not. + + If the tag is inherited, it will return `True`, if it is owned by the object itself, it will return `False`. + + **Note**. The return value is optional, only when the tag is associated with an object, and called from the + object, the return value will be present. Otherwise, it will be empty. + + Returns: + Optional[bool]: + True if the tag is inherited, false if it is owned by the object itself. Empty if the + tag is not associated with any object. + """ + return self._tag_dto.inherited() + + def audit_info(self) -> AuditDTO: + """ + Retrieve The audit information of the entity. + + Returns: + AuditDTO: The audit information of the entity. + """ + return self._tag_dto.audit_info() + + def associated_objects(self) -> Tag.AssociatedObjects: + """ + The associated objects of the tag. + + Returns: + AssociatedObjects: The associated objects of the tag. + """ + return self + + def objects(self) -> list[MetadataObject]: + """ + Retrieve The list of objects that are associated with this tag. Review Comment: Inconsistent capitalization in documentation. The word 'The' at the beginning of 'Retrieve The list' should be lowercase to be consistent with standard documentation style. Change to 'Retrieve the list'. ```suggestion Retrieve the list of objects that are associated with this tag. ``` ########## clients/client-python/gravitino/exceptions/base.py: ########## @@ -177,6 +177,10 @@ class TagAlreadyExistsException(AlreadyExistsException): """An exception thrown when a tag with specified name already associated to a metadata object.""" Review Comment: Incorrect exception docstring. TagAlreadyExistsException should describe when a tag already exists in the system (during creation), not when it's associated with an object. The docstring should be: "An exception thrown when a tag with specified name already exists." ```suggestion """An exception thrown when a tag with specified name already exists.""" ``` ########## clients/client-python/gravitino/client/generic_tag.py: ########## @@ -0,0 +1,150 @@ +# 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 typing import Optional + +from gravitino.api.metadata_object import MetadataObject +from gravitino.api.tag.tag import Tag +from gravitino.dto.audit_dto import AuditDTO +from gravitino.dto.responses.metadata_object_list_response import ( + MetadataObjectListResponse, +) +from gravitino.dto.tag_dto import TagDTO +from gravitino.exceptions.handlers.error_handler import ErrorHandler +from gravitino.exceptions.handlers.error_handlers import ErrorHandlers +from gravitino.rest.rest_utils import encode_string +from gravitino.utils import HTTPClient +from gravitino.utils.http_client import Response + + +class GenericTag(Tag, Tag.AssociatedObjects): + """Represents a generic tag.""" + + API_LIST_OBJECTS_ENDPOINT = "api/metalakes/{}/tags/{}/objects" + + def __init__( + self, + metalake: str, + tag_dto: TagDTO, + client: HTTPClient, + ) -> None: + self._metalake = metalake + self._tag_dto = tag_dto + self._client = client + + def __eq__(self, other: object) -> bool: + if not isinstance(other, GenericTag): + return False + + return self._tag_dto == other._tag_dto + + def __hash__(self) -> int: + return hash(self._tag_dto) + + def name(self) -> str: + """Get the name of the tag. + + Returns: + str: The name of the tag. + """ + return self._tag_dto.name() + + def comment(self) -> str: + """Get the comment of the tag. + + Returns: + str: The comment of the tag. + """ + return self._tag_dto.comment() + + def properties(self) -> dict[str, str]: + """Get the properties of the tag. + + Returns: + Dict[str, str]: The properties of the tag. + """ + return self._tag_dto.properties() + + def inherited(self) -> Optional[bool]: + """Check if the tag is inherited from a parent object or not. + + If the tag is inherited, it will return `True`, if it is owned by the object itself, it will return `False`. + + **Note**. The return value is optional, only when the tag is associated with an object, and called from the + object, the return value will be present. Otherwise, it will be empty. + + Returns: + Optional[bool]: + True if the tag is inherited, false if it is owned by the object itself. Empty if the + tag is not associated with any object. + """ + return self._tag_dto.inherited() + + def audit_info(self) -> AuditDTO: + """ + Retrieve The audit information of the entity. Review Comment: Inconsistent capitalization in documentation. The word 'The' at the beginning of 'Retrieve The audit' should be lowercase to be consistent with standard documentation style. Change to 'Retrieve the audit'. ```suggestion Retrieve the audit information of the entity. ``` ########## clients/client-python/gravitino/dto/responses/metadata_object_list_response.py: ########## @@ -0,0 +1,59 @@ +# 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 dataclasses import dataclass, field + +from dataclasses_json import config + +from gravitino.dto.metadata_object_dto import MetadataObjectDTO +from gravitino.dto.responses.base_response import BaseResponse +from gravitino.utils.precondition import Precondition + + +@dataclass +class MetadataObjectListResponse(BaseResponse): + """Represents a response containing a list of metadata objects.""" + + _metadata_objects: list[MetadataObjectDTO] = field( + metadata=config(field_name="metadataObjects") + ) + + def metadata_objects(self) -> list[MetadataObjectDTO]: + """ + Retrieve the list of metadata objects. + + Returns: + list[MetadataObjectDTO]: The list of metadata objects. + """ + return self._metadata_objects + + def validate(self) -> None: + """ + Validates the response data. + """ + super().validate() + + for metadata_object in self._metadata_objects: + Precondition.check_argument( + metadata_object is not None + and metadata_object.type() is not None + and (name := metadata_object.name()) is not None + and name.strip() != "", + "metadataObject must not be null and it's field cannot null or empty", Review Comment: Grammar error: "it's" should be "its" (possessive form, not contraction). The error message should read: "metadataObject must not be null and its field cannot be null or empty" ```suggestion "metadataObject must not be null and its field cannot null or empty", ``` ########## clients/client-python/gravitino/api/tag/tag.py: ########## @@ -117,3 +117,28 @@ def associated_objects(self) -> AssociatedObjects: raise UnsupportedOperationException( "The associated_objects method is not supported." ) + + class AssociatedObjects(ABC): + """The interface of the associated objects of the tag.""" + + def count(self) -> int: + """ + Retrive the number of objects that are associated with this Tag + + Returns: + int: The number of objects that are associated with this Tag + """ + return 0 if (s := self.objects()) is None else len(s) + + @abstractmethod + def objects(self) -> list[MetadataObject]: + """ + Retrieve The list of objects that are associated with this tag. Review Comment: Inconsistent capitalization in documentation. The word 'The' at the beginning of 'Retrieve The list' should be lowercase to be consistent with standard documentation style. Change to 'Retrieve the list'. ```suggestion Retrieve the list of objects that are associated with this tag. ``` ########## clients/client-python/gravitino/api/tag/tag.py: ########## @@ -117,3 +117,28 @@ def associated_objects(self) -> AssociatedObjects: raise UnsupportedOperationException( "The associated_objects method is not supported." ) + + class AssociatedObjects(ABC): + """The interface of the associated objects of the tag.""" + + def count(self) -> int: + """ + Retrive the number of objects that are associated with this Tag Review Comment: Spelling error: 'Retrive' should be 'Retrieve'. ```suggestion Retrieve the number of objects that are associated with this Tag ``` ########## clients/client-python/gravitino/client/gravitino_metalake.py: ########## @@ -71,6 +75,7 @@ class GravitinoMetalake( API_METALAKES_CATALOGS_PATH = "api/metalakes/{}/catalogs/{}" API_METALAKES_JOB_TEMPLATES_PATH = "api/metalakes/{}/jobs/templates" API_METALAKES_JOB_RUNS_PATH = "api/metalakes/{}/jobs/runs" + API_METALAKES_TAGS_PATH = "api/metalakes/{}/tags/{}" Review Comment: The API_METALAKES_TAGS_PATH constant has two placeholders but should only have one. According to the Java implementation, the path should be "api/metalakes/{}/tags" with only the metalake name placeholder. The tag name should be appended when needed (e.g., in get_tag method). ```suggestion API_METALAKES_TAGS_PATH = "api/metalakes/{}/tags" ``` ########## clients/client-python/tests/unittests/dto/requests/test_tag_create_request.py: ########## @@ -0,0 +1,44 @@ +# 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 + +import json as _json +import unittest + +from gravitino.dto.requests import TagCreateRequest + + +class TestTagCreateRequest(unittest.TestCase): + def tag_create_request_serde(self) -> None: Review Comment: Test method name is missing the 'test_' prefix. The method should be named 'test_tag_create_request_serde' to be discovered and executed by the test runner. ```suggestion def test_tag_create_request_serde(self) -> None: ``` ########## clients/client-python/gravitino/dto/tag_dto.py: ########## @@ -0,0 +1,152 @@ +# 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 dataclasses import dataclass, field +from typing import Optional + +from dataclasses_json import config, dataclass_json + +from gravitino.api.tag.tag import Tag +from gravitino.dto.audit_dto import AuditDTO + + +@dataclass_json +@dataclass +class TagDTO(Tag): + """Represents a Tag Data Transfer Object (DTO).""" + + _name: str = field(metadata=config(field_name="name")) + _comment: str = field(metadata=config(field_name="comment")) + _properties: dict[str, str] = field(metadata=config(field_name="properties")) + + _audit: AuditDTO = field(default=None, metadata=config(field_name="audit")) + _inherited: Optional[bool] = field( + default=None, metadata=config(field_name="inherited") + ) + + def __eq__(self, other: object): + if not isinstance(other, TagDTO): + return False + return ( + self._name == other._name + and self._comment == other._comment + and self._properties == other._properties + and self._audit == other._audit + ) + + def __hash__(self) -> int: + return hash( + ( + self._name, + self._comment, + frozenset(self._properties.items()) if self._properties else None, + self._audit, + ) + ) + + @staticmethod + def builder() -> TagDTO.Builder: + return TagDTO.Builder() + + def name(self) -> str: + """Get the name of the tag. + + Returns: + str: The name of the tag. + """ + return self._name + + def comment(self): Review Comment: Missing return type hint. The comment method should have a return type annotation of '-> str' to be consistent with the other methods in the class and improve type safety. ```suggestion def comment(self) -> str: ``` ########## clients/client-python/gravitino/dto/responses/tag_response.py: ########## @@ -0,0 +1,70 @@ +# 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 dataclasses import dataclass, field + +from dataclasses_json import config + +from gravitino.dto.responses.base_response import BaseResponse +from gravitino.dto.tag_dto import TagDTO +from gravitino.utils.precondition import Precondition + + +@dataclass +class TagNamesListResponse(BaseResponse): + """Represents a response for a Tag Names List request.""" + + _tags: list[str] = field(default_factory=list, metadata=config(field_name="names")) + + def tag_names(self) -> list[str]: + return self._tags + + def validate(self) -> None: + Precondition.check_argument( + self._tags is not None, "Tag Names List response should have tags" + ) + + for tag_name in self._tags: + Precondition.check_string_not_empty( + tag_name, "Tag Names List response should have non-empty tag names" + ) + + +@dataclass +class TagListResponse(BaseResponse): + """Represents a response for a Tag List request.""" + + _tags: list[TagDTO] = field( + default_factory=list, metadata=config(field_name="tags") + ) + + def tags(self) -> list[TagDTO]: + return self._tags + + +@dataclass +class TagResponse(BaseResponse): + """Represents a response for a tag.""" + + _tag: TagDTO = field(default=None, metadata=config(field_name="tag")) + + def validate(self) -> None: + Precondition.check_argument( + self._tag is not None, "Tag response should have a tag" + ) Review Comment: Missing getter method for the tag field. The TagResponse class is missing a tag() method to access the _tag field. This is needed at line 581 and 617 in gravitino_metalake.py where tag_resp.tag() is called. Add a method: def tag(self) -> TagDTO: return self._tag ########## clients/client-python/tests/unittests/test_generic_tag.py: ########## @@ -0,0 +1,230 @@ +# 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 + +import json as _json +import unittest +from unittest.mock import MagicMock + +from gravitino.api.metadata_object import MetadataObject +from gravitino.client.generic_tag import GenericTag +from gravitino.dto.audit_dto import AuditDTO +from gravitino.dto.metadata_object_dto import MetadataObjectDTO +from gravitino.dto.tag_dto import TagDTO +from gravitino.exceptions.base import InternalError, NoSuchMetalakeException +from gravitino.utils import HTTPClient +from gravitino.utils.http_client import Response + + +class TestGenericTag(unittest.TestCase): + METALAKE = "metalake1" + + TAG_DTO = ( + TagDTO.Builder() + .name("tag1") + .comment("comment1") + .properties({"key1": "value1"}) + .inherited(True) + .audit_info(AuditDTO(_creator="test", _create_time="2022-01-01T00:00:00Z")) + .build() + ) + + @classmethod + def setUpClass(cls): + cls._rest_client = HTTPClient("http://localhost:8090";) + cls._metalake_name = "metalake_demo" + + def test_create_generic_tag(self): + generic_tag = GenericTag(self.METALAKE, self.TAG_DTO, self._rest_client) + self.assertEqual("tag1", generic_tag.name()) + self.assertEqual("comment1", generic_tag.comment()) + self.assertEqual({"key1": "value1"}, generic_tag.properties()) + self.assertEqual(True, generic_tag.inherited()) + self.assertEqual( + AuditDTO(_creator="test", _create_time="2022-01-01T00:00:00Z"), + generic_tag.audit_info(), + ) + + def test_generic_tag_associated_objects(self): + # Test normal situation + response_body = { + "code": 0, + "metadataObjects": [ + { + "fullName": "catalog1", + "type": "catalog", + }, + { + "fullName": "catalog1.schema1", + "type": "schema", + }, + { + "fullName": "catalog1.schema1.table1", + "type": "table", + }, + { + "fullName": "catalog1.schema1.table1.column1", + "type": "column", + }, + ], + } + + expected_associated_objects = [ + MetadataObjectDTO.builder() + .full_name("catalog1") + .type(MetadataObject.Type.CATALOG) + .build(), + MetadataObjectDTO.builder() + .full_name("catalog1.schema1") + .type(MetadataObject.Type.SCHEMA) + .build(), + MetadataObjectDTO.builder() + .full_name("catalog1.schema1.table1") + .type(MetadataObject.Type.TABLE) + .build(), + MetadataObjectDTO.builder() + .full_name("catalog1.schema1.table1.column1") + .type(MetadataObject.Type.COLUMN) + .build(), + ] + generic_tag = TestGenericTagEntity( + self.METALAKE, self.TAG_DTO, self._rest_client, response_body + ) + objects = generic_tag.associated_objects().objects() + self.assertEqual(4, len(objects)) + self.assertEqual(4, generic_tag.count()) + + for i, v in enumerate(objects): + self.assertEqual(v, expected_associated_objects[i]) + self.assertEqual(v.full_name(), expected_associated_objects[i].full_name()) + self.assertEqual(v.type(), expected_associated_objects[i].type()) + self.assertEqual(v.parent(), expected_associated_objects[i].parent()) + self.assertEqual(v.name(), expected_associated_objects[i].name()) + + # Test return empty array + generic_tag = TestGenericTagEntity( + self.METALAKE, + self.TAG_DTO, + self._rest_client, + { + "code": 0, + "metadataObjects": [], + }, + ) + objects = generic_tag.associated_objects().objects() + self.assertEqual(0, len(objects)) + self.assertEqual(0, generic_tag.count()) + + # Test throw NoSuchMetalakeException + with self.assertRaises(NoSuchMetalakeException): + generic_tag = TestGenericTagEntity( + self.METALAKE, + self.TAG_DTO, + self._rest_client, + { + "code": 0, + "metadataObjects": [], + }, + NoSuchMetalakeException, + ) + generic_tag.associated_objects().objects() + + # Test throw internal error + with self.assertRaises(InternalError): + generic_tag = TestGenericTagEntity( + self.METALAKE, + self.TAG_DTO, + self._rest_client, + { + "code": 0, + "metadataObjects": [], + }, + InternalError, + ) + generic_tag.associated_objects().objects() + + def test_hash_and_equal(self) -> None: + tag_dto1 = ( + TagDTO.Builder() + .name("tag1") + .comment("comment1") + .properties({"key1": "value1"}) + .inherited(True) + .audit_info(AuditDTO(_creator="test", _create_time="2022-01-01T00:00:00Z")) + .build() + ) + generic_tag1 = GenericTag(self.METALAKE, tag_dto1, self._rest_client) + + tag_dto2 = ( + TagDTO.Builder() + .name("tag1") + .comment("comment1") + .properties({"key1": "value1"}) + .inherited(True) + .audit_info(AuditDTO(_creator="test", _create_time="2022-01-01T00:00:00Z")) + .build() + ) + generic_tag2 = GenericTag(self.METALAKE, tag_dto2, self._rest_client) + + tag_dto3 = ( + TagDTO.Builder() + .name("tag1") + .comment("comment1") + .properties({"key1": "value1"}) + .inherited(True) + .audit_info(AuditDTO(_creator="test", _create_time="2022-01-02T00:00:00Z")) + .build() + ) + generic_tag3 = GenericTag(self.METALAKE, tag_dto3, self._rest_client) + + self.assertEqual(generic_tag1, generic_tag2) + self.assertEqual(hash(generic_tag1), hash(generic_tag2)) + + self.assertNotEqual(generic_tag1, generic_tag3) + self.assertNotEqual(hash(generic_tag1), hash(generic_tag3)) + + +class TestGenericTagEntity(GenericTag): Review Comment: The class 'TestGenericTagEntity' does not override ['__eq__'](1), but adds the new attribute [dump_object](2). The class 'TestGenericTagEntity' does not override ['__eq__'](1), but adds the new attribute [throw_error](3). -- 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]
