roryqi commented on code in PR #12442:
URL: https://github.com/apache/gravitino/pull/12442#discussion_r3802554798
##########
clients/client-python/gravitino/dto/requests/tag_associate_request.py:
##########
@@ -27,45 +28,162 @@
@dataclass_json
@dataclass
-class TagsAssociateRequest(RESTRequest):
- """
- Represents a request to associate tags.
- """
+class TagValuePairRequest(RESTRequest):
+ """Represents a tag assignment value pair request."""
+
+ _name: str = field(metadata=config(field_name="name"))
+ _value: Optional[str] = field(default=None,
metadata=config(field_name="value"))
- _tags_to_add: list[str] = field(metadata=config(field_name="tagsToAdd"))
- _tags_to_remove: list[str] =
field(metadata=config(field_name="tagsToRemove"))
+ @property
+ def name(self) -> str:
+ """Gets the tag name."""
+ return self._name
@property
- def tags_to_add(self) -> list[str]:
- """
- Gets the tags to add.
- """
+ def value(self) -> Optional[str]:
+ """Gets the tag assignment value."""
+ return self._value
+
+ def validate(self) -> None:
+ """Validates the request."""
+ Precondition.check_argument(
+ StringUtils.is_not_blank(self._name),
+ "Tag name must not be null or empty",
+ )
+ if self._value is not None:
+ Precondition.check_argument(
+ self._value.strip() != "",
+ "Tag value must not be empty",
+ )
+ Precondition.check_argument(
+ len(self._value) <= 256,
+ "Tag value must not be longer than 256 characters",
+ )
+
+
+@dataclass_json
+@dataclass
+class TagNamesAssociateRequest(RESTRequest):
+ """Represents a request to associate tag names."""
+
+ _tags_to_add: Optional[list[str]] = field(
+ default=None, metadata=config(field_name="tagsToAdd")
+ )
+ _tags_to_remove: Optional[list[str]] = field(
+ default=None, metadata=config(field_name="tagsToRemove")
+ )
+
+ @property
+ def tags_to_add(self) -> Optional[list[str]]:
+ """Gets the tags to add."""
return self._tags_to_add
@property
- def tags_to_remove(self) -> list[str]:
- """
- Gets the tags to remove.
- """
+ def tags_to_remove(self) -> Optional[list[str]]:
+ """Gets the tags to remove."""
return self._tags_to_remove
def validate(self) -> None:
- """
- Validates the request.
- """
+ """Validates the request."""
Precondition.check_argument(
self._tags_to_add is not None or self._tags_to_remove is not None,
"tagsToAdd and tagsToRemove cannot both be null",
)
- self._validate_tags(self._tags_to_add, "tagsToAdd")
- self._validate_tags(self._tags_to_remove, "tagsToRemove")
+ self._validate_tag_names(self._tags_to_add, "tagsToAdd")
+ self._validate_tag_names(self._tags_to_remove, "tagsToRemove")
- def _validate_tags(self, tags: list[str] | None, field_name: str) -> None:
- if tags is None:
+ def _validate_tag_names(
+ self, tag_names: Optional[list[str]], field_name: str
+ ) -> None:
+ if tag_names is None:
return
Precondition.check_argument(
- all(StringUtils.is_not_blank(tag) for tag in tags),
+ all(StringUtils.is_not_blank(tag_name) for tag_name in tag_names),
f"{field_name} must not contain null or empty tag names",
)
+
+
+@dataclass_json
+@dataclass
+class TagsAssociateRequest(RESTRequest):
Review Comment:
Done in 689a1e3e9f. `TagsAssociateRequest` is restored as the legacy
string-array DTO, and `assign_tags` now uses the separately named
`TagValuesAssociateRequest`.
##########
clients/client-python/gravitino/dto/requests/tag_associate_request.py:
##########
@@ -27,45 +28,149 @@
@dataclass_json
@dataclass
-class TagsAssociateRequest(RESTRequest):
- """
- Represents a request to associate tags.
- """
+class TagValuePairRequest(RESTRequest):
+ """Represents a tag assignment value pair request."""
+
+ _name: str = field(metadata=config(field_name="name"))
+ _value: Optional[str] = field(default=None,
metadata=config(field_name="value"))
+
+ @property
+ def name(self) -> str:
+ """Gets the tag name."""
+ return self._name
+
+ @property
+ def value(self) -> Optional[str]:
+ """Gets the tag assignment value."""
+ return self._value
+
+ def validate(self) -> None:
+ """Validates the request."""
+ Precondition.check_argument(
+ StringUtils.is_not_blank(self._name),
+ "Tag name must not be null or empty",
+ )
+ if self._value is not None:
+ Precondition.check_argument(
+ self._value.strip() != "",
+ "Tag value must not be empty",
+ )
+ Precondition.check_argument(
+ len(self._value) <= 256,
+ "Tag value must not be longer than 256 characters",
+ )
- _tags_to_add: list[str] = field(metadata=config(field_name="tagsToAdd"))
- _tags_to_remove: list[str] =
field(metadata=config(field_name="tagsToRemove"))
+
+@dataclass_json
+@dataclass
+class TagNamesAssociateRequest(RESTRequest):
+ """Represents a request to associate tag names."""
+
+ _tags_to_add: Optional[list[str]] = field(
+ default=None, metadata=config(field_name="tagsToAdd")
+ )
+ _tags_to_remove: Optional[list[str]] = field(
+ default=None, metadata=config(field_name="tagsToRemove")
+ )
@property
- def tags_to_add(self) -> list[str]:
- """
- Gets the tags to add.
- """
+ def tags_to_add(self) -> Optional[list[str]]:
+ """Gets the tags to add."""
return self._tags_to_add
@property
- def tags_to_remove(self) -> list[str]:
- """
- Gets the tags to remove.
- """
+ def tags_to_remove(self) -> Optional[list[str]]:
+ """Gets the tags to remove."""
return self._tags_to_remove
def validate(self) -> None:
- """
- Validates the request.
- """
+ """Validates the request."""
Precondition.check_argument(
- self._tags_to_add is not None or self._tags_to_remove is not None,
- "tagsToAdd and tagsToRemove cannot both be null",
+ bool(self._tags_to_add) or bool(self._tags_to_remove),
+ "tagsToAdd and tagsToRemove cannot both be null or empty",
Review Comment:
Done. The legacy name-array DTO is restored as `TagsAssociateRequest` and
keeps the original null-only validation, so `associate_tags([], [])` remains
valid.
##########
clients/client-python/gravitino/dto/requests/tag_associate_request.py:
##########
@@ -27,45 +28,149 @@
@dataclass_json
@dataclass
-class TagsAssociateRequest(RESTRequest):
- """
- Represents a request to associate tags.
- """
+class TagValuePairRequest(RESTRequest):
+ """Represents a tag assignment value pair request."""
+
+ _name: str = field(metadata=config(field_name="name"))
+ _value: Optional[str] = field(default=None,
metadata=config(field_name="value"))
+
+ @property
+ def name(self) -> str:
+ """Gets the tag name."""
+ return self._name
+
+ @property
+ def value(self) -> Optional[str]:
+ """Gets the tag assignment value."""
+ return self._value
+
+ def validate(self) -> None:
+ """Validates the request."""
+ Precondition.check_argument(
+ StringUtils.is_not_blank(self._name),
+ "Tag name must not be null or empty",
+ )
+ if self._value is not None:
+ Precondition.check_argument(
+ self._value.strip() != "",
+ "Tag value must not be empty",
+ )
+ Precondition.check_argument(
+ len(self._value) <= 256,
+ "Tag value must not be longer than 256 characters",
+ )
- _tags_to_add: list[str] = field(metadata=config(field_name="tagsToAdd"))
- _tags_to_remove: list[str] =
field(metadata=config(field_name="tagsToRemove"))
+
+@dataclass_json
+@dataclass
+class TagNamesAssociateRequest(RESTRequest):
+ """Represents a request to associate tag names."""
+
+ _tags_to_add: Optional[list[str]] = field(
+ default=None, metadata=config(field_name="tagsToAdd")
+ )
+ _tags_to_remove: Optional[list[str]] = field(
+ default=None, metadata=config(field_name="tagsToRemove")
+ )
@property
- def tags_to_add(self) -> list[str]:
- """
- Gets the tags to add.
- """
+ def tags_to_add(self) -> Optional[list[str]]:
+ """Gets the tags to add."""
return self._tags_to_add
@property
- def tags_to_remove(self) -> list[str]:
- """
- Gets the tags to remove.
- """
+ def tags_to_remove(self) -> Optional[list[str]]:
+ """Gets the tags to remove."""
return self._tags_to_remove
def validate(self) -> None:
- """
- Validates the request.
- """
+ """Validates the request."""
Precondition.check_argument(
- self._tags_to_add is not None or self._tags_to_remove is not None,
- "tagsToAdd and tagsToRemove cannot both be null",
+ bool(self._tags_to_add) or bool(self._tags_to_remove),
+ "tagsToAdd and tagsToRemove cannot both be null or empty",
)
- self._validate_tags(self._tags_to_add, "tagsToAdd")
- self._validate_tags(self._tags_to_remove, "tagsToRemove")
+ self._validate_tag_names(self._tags_to_add, "tagsToAdd")
+ self._validate_tag_names(self._tags_to_remove, "tagsToRemove")
- def _validate_tags(self, tags: list[str] | None, field_name: str) -> None:
- if tags is None:
+ def _validate_tag_names(
+ self, tag_names: Optional[list[str]], field_name: str
+ ) -> None:
+ if tag_names is None:
return
Precondition.check_argument(
- all(StringUtils.is_not_blank(tag) for tag in tags),
+ all(StringUtils.is_not_blank(tag_name) for tag_name in tag_names),
f"{field_name} must not contain null or empty tag names",
)
+
+
+@dataclass_json
+@dataclass
+class TagsAssociateRequest(RESTRequest):
+ """Represents a request to associate tags."""
+
+ _tags_to_add: Optional[list[TagValuePairRequest]] = field(
+ default=None, metadata=config(field_name="tagsToAdd")
+ )
+ _tags_to_remove: Optional[list[TagValuePairRequest]] = field(
+ default=None, metadata=config(field_name="tagsToRemove")
+ )
+
+ def __post_init__(self) -> None:
+ self._tags_to_add = self._normalize_pairs(self._tags_to_add)
+ self._tags_to_remove = self._normalize_pairs(self._tags_to_remove)
+
+ @property
+ def tags_to_add(self) -> Optional[list[TagValuePairRequest]]:
+ """Gets the tags to add."""
+ return self._tags_to_add
+
+ @property
+ def tags_to_remove(self) -> Optional[list[TagValuePairRequest]]:
+ """Gets the tags to remove."""
+ return self._tags_to_remove
+
+ def validate(self) -> None:
+ """Validates the request."""
+ Precondition.check_argument(
+ bool(self._tags_to_add) or bool(self._tags_to_remove),
+ "tagsToAdd and tagsToRemove cannot both be null or empty",
+ )
+
+ self._validate_pairs(self._tags_to_add, "tagsToAdd")
+ self._validate_pairs(self._tags_to_remove, "tagsToRemove")
Review Comment:
Done. The value-pair request validation rejects an exact tag/value pair
appearing in both add and remove before sending the request.
##########
clients/client-python/gravitino/api/tag/supports_tags.py:
##########
@@ -61,6 +61,23 @@ def get_tag(self, name: str) -> Tag:
"""
pass
+ @abstractmethod
Review Comment:
Done in 689a1e3e9f. The new APIs now have compatibility defaults:
`assign_tags` raises `UnsupportedOperationException`, and the new `Tag` value
accessors return `None`. Added a compatibility test for legacy implementations.
##########
clients/client-python/gravitino/dto/tag_dto.py:
##########
@@ -33,6 +33,12 @@ class TagDTO(Tag):
_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"))
+ _allowed_values: Optional[list[str]] = field(
Review Comment:
Done in 689a1e3e9f. The new fields now come after the existing positional
fields, and the builder constructs `TagDTO` with keyword arguments. Added a
positional-constructor compatibility test.
--
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]
