This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-0.7
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-0.7 by this push:
new 76dd6cc49 [#4968] improvement(api): Unify the modification behavior of
the comment field (#5288)
76dd6cc49 is described below
commit 76dd6cc497e0f06403569bb2f82bede55351bfdf
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Oct 28 14:49:48 2024 +0800
[#4968] improvement(api): Unify the modification behavior of the comment
field (#5288)
### What changes were proposed in this pull request?
- deprecated the removeComment change
- the new comment of updateComment supports null and empty string
### Why are the changes needed?
Fix: #4968
### Does this PR introduce _any_ user-facing change?
N/A
### How was this patch tested?
Pass existing tests
Co-authored-by: Ricco Chen <[email protected]>
Co-authored-by: chenzeping.ricco <[email protected]>
---
.../java/org/apache/gravitino/file/FilesetChange.java | 7 ++++++-
.../catalog/hadoop/TestHadoopCatalogOperations.java | 2 +-
.../catalog/hadoop/integration/test/HadoopCatalogIT.java | 2 +-
.../catalog/kafka/integration/test/CatalogKafkaIT.java | 4 ++++
.../org/apache/gravitino/client/TestFilesetCatalog.java | 2 +-
.../gravitino/client/integration/test/CatalogIT.java | 6 +++++-
.../gravitino/client/integration/test/MetalakeIT.java | 8 +++++++-
.../apache/gravitino/client/integration/test/TagIT.java | 8 ++++++++
clients/client-python/gravitino/api/fileset_change.py | 11 +++++++++--
.../client-python/gravitino/catalog/fileset_catalog.py | 2 +-
.../gravitino/dto/requests/catalog_update_request.py | 4 ++--
.../gravitino/dto/requests/fileset_update_request.py | 15 +++++++--------
.../gravitino/dto/requests/metalake_update_request.py | 9 ++-------
clients/client-python/tests/integration/test_catalog.py | 8 ++++++++
.../tests/integration/test_fileset_catalog.py | 2 +-
clients/client-python/tests/integration/test_metalake.py | 8 ++++++++
.../gravitino/dto/requests/CatalogUpdateRequest.java | 12 ++----------
.../gravitino/dto/requests/FilesetUpdateRequest.java | 14 +++-----------
.../gravitino/dto/requests/MetalakeUpdateRequest.java | 12 ++----------
.../apache/gravitino/dto/requests/TableUpdateRequest.java | 12 ++----------
.../apache/gravitino/dto/requests/TagUpdateRequest.java | 6 ++----
.../apache/gravitino/dto/requests/TopicUpdateRequest.java | 12 ++----------
.../gravitino/catalog/TestFilesetOperationDispatcher.java | 2 +-
docs/manage-fileset-metadata-using-gravitino.md | 14 +++++++-------
.../gravitino/server/web/rest/TestFilesetOperations.java | 4 ++--
25 files changed, 94 insertions(+), 92 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java
b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java
index 2df992ece..6b79aed41 100644
--- a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java
+++ b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java
@@ -73,7 +73,9 @@ public interface FilesetChange {
* Creates a new fileset change to remove comment from the fileset.
*
* @return The fileset change.
+ * @deprecated Use {@link #updateComment(String)} with null value as the
argument instead.
*/
+ @Deprecated
static FilesetChange removeComment() {
return RemoveComment.getInstance();
}
@@ -310,7 +312,10 @@ public interface FilesetChange {
}
}
- /** A fileset change to remove comment from the fileset. */
+ /**
+ * A fileset change to remove comment from the fileset. Use {@link
UpdateFilesetComment} with null
+ * value as the argument instead.
+ */
final class RemoveComment implements FilesetChange {
private static final RemoveComment INSTANCE = new RemoveComment();
diff --git
a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
index 2b89180a8..9b5b61f27 100644
---
a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
+++
b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
@@ -770,7 +770,7 @@ public class TestHadoopCatalogOperations {
String name = "fileset27";
Fileset fileset = createFileset(name, schemaName, comment,
Fileset.Type.MANAGED, null, null);
- FilesetChange change1 = FilesetChange.removeComment();
+ FilesetChange change1 = FilesetChange.updateComment(null);
try (SecureHadoopCatalogOperations ops = new
SecureHadoopCatalogOperations(store)) {
ops.initialize(Maps.newHashMap(), randomCatalogInfo(),
HADOOP_PROPERTIES_METADATA);
NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName,
name);
diff --git
a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java
b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java
index b272bd7a8..6cd10cbf2 100644
---
a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java
+++
b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java
@@ -575,7 +575,7 @@ public class HadoopCatalogIT extends BaseIT {
catalog
.asFilesetCatalog()
.alterFileset(
- NameIdentifier.of(schemaName, filesetName),
FilesetChange.removeComment());
+ NameIdentifier.of(schemaName, filesetName),
FilesetChange.updateComment(null));
assertFilesetExists(filesetName);
// verify fileset is updated
diff --git
a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java
b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java
index dc91a3dda..52ebb8dab 100644
---
a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java
+++
b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java
@@ -172,6 +172,10 @@ public class CatalogKafkaIT extends BaseIT {
Assertions.assertEquals("new comment", alteredCatalog.comment());
Assertions.assertFalse(alteredCatalog.properties().containsKey("key1"));
+ Catalog updateNullComment =
+ metalake.alterCatalog(catalogName, CatalogChange.updateComment(null));
+ Assertions.assertNull(updateNullComment.comment());
+
// test drop catalog
boolean dropped = metalake.dropCatalog(catalogName, true);
Assertions.assertTrue(dropped);
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java
index 446af40ed..f45adfab5 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java
@@ -378,7 +378,7 @@ public class TestFilesetCatalog extends TestBase {
assertFileset(mockFileset3, res3);
// Test remove fileset comment
- FilesetUpdateRequest req4 = new
FilesetUpdateRequest.RemoveFilesetCommentRequest();
+ FilesetUpdateRequest req4 = new
FilesetUpdateRequest.UpdateFilesetCommentRequest(null);
FilesetDTO mockFileset4 =
mockFilesetDTO("new name", Fileset.Type.MANAGED, null, "mock
location", ImmutableMap.of());
FilesetResponse resp4 = new FilesetResponse(mockFileset4);
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java
index 045c0ad69..bb6394b6c 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/CatalogIT.java
@@ -221,7 +221,7 @@ public class CatalogIT extends BaseIT {
CatalogNotInUseException.class,
() ->
filesetOps.alterFileset(
- NameIdentifier.of("dummy", "dummy"),
FilesetChange.removeComment()));
+ NameIdentifier.of("dummy", "dummy"),
FilesetChange.updateComment(null)));
Assertions.assertTrue(metalake.dropCatalog(catalogName), "catalog should
be dropped");
Assertions.assertFalse(metalake.dropCatalog(catalogName), "catalog should
be non-existent");
@@ -399,6 +399,10 @@ public class CatalogIT extends BaseIT {
metalake.alterCatalog(catalogName, CatalogChange.updateComment("new
catalog comment"));
Assertions.assertEquals("new catalog comment", updatedCatalog.comment());
+ Catalog updateNullComment =
+ metalake.alterCatalog(catalogName, CatalogChange.updateComment(null));
+ Assertions.assertNull(updateNullComment.comment());
+
metalake.disableCatalog(catalogName);
metalake.dropCatalog(catalogName);
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java
index 3a6506464..7911f02cd 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/MetalakeIT.java
@@ -22,6 +22,7 @@ import static org.apache.gravitino.Metalake.PROPERTY_IN_USE;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -219,6 +220,11 @@ public class MetalakeIT extends BaseIT {
new MetalakeChange[] {MetalakeChange.updateComment("new metalake
comment")};
GravitinoMetalake updatedMetalake = client.alterMetalake(metalakeNameA,
changes);
assertEquals("new metalake comment", updatedMetalake.comment());
+
+ GravitinoMetalake updateNullComment =
+ client.alterMetalake(metalakeNameA,
MetalakeChange.updateComment(null));
+ assertNull(updateNullComment.comment());
+
assertTrue(client.dropMetalake(metalakeNameA, true));
}
@@ -317,7 +323,7 @@ public class MetalakeIT extends BaseIT {
MetalakeNotInUseException.class,
() ->
filesetOps.alterFileset(
- NameIdentifier.of("dummy", "dummy"),
FilesetChange.removeComment()));
+ NameIdentifier.of("dummy", "dummy"),
FilesetChange.updateComment(null)));
Assertions.assertThrows(
NonEmptyMetalakeException.class, () ->
client.dropMetalake(metalakeName));
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java
index bd95f2ae3..4278d3f88 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/TagIT.java
@@ -197,6 +197,14 @@ public class TagIT extends BaseIT {
Assertions.assertEquals(tag2, tag3);
}
+ @Test
+ public void testNullableComment() {
+ String tagName = GravitinoITUtils.genRandomName("tag_it_tag");
+ metalake.createTag(tagName, "comment", Collections.emptyMap());
+ Tag alteredTag6 = metalake.alterTag(tagName,
TagChange.updateComment(null));
+ Assertions.assertNull(alteredTag6.comment());
+ }
+
@Test
public void testCreateAndAlterTag() {
String tagName = GravitinoITUtils.genRandomName("tag_it_tag");
diff --git a/clients/client-python/gravitino/api/fileset_change.py
b/clients/client-python/gravitino/api/fileset_change.py
index 2be9bc568..b723a8a29 100644
--- a/clients/client-python/gravitino/api/fileset_change.py
+++ b/clients/client-python/gravitino/api/fileset_change.py
@@ -81,8 +81,11 @@ class FilesetChange(ABC):
Returns:
The fileset change.
+
+ Deprecated:
+ Please use `update_comment(str)` with null value as the argument
instead.
"""
- return FilesetChange.RemoveComment()
+ return FilesetChange.UpdateFilesetComment(None)
@dataclass
class RenameFileset:
@@ -279,7 +282,11 @@ class FilesetChange(ABC):
@dataclass
class RemoveComment:
- """A fileset change to remove comment from the fileset."""
+ """A fileset change to remove comment from the fileset.
+
+ Deprecated:
+ Please use `UpdateFilesetComment(str)` with null value as the
argument instead.
+ """
def __eq__(self, other) -> bool:
"""Compares this RemoveComment instance with another object for
equality.
diff --git a/clients/client-python/gravitino/catalog/fileset_catalog.py
b/clients/client-python/gravitino/catalog/fileset_catalog.py
index cf91dbdfb..ffa252e62 100644
--- a/clients/client-python/gravitino/catalog/fileset_catalog.py
+++ b/clients/client-python/gravitino/catalog/fileset_catalog.py
@@ -319,5 +319,5 @@ class FilesetCatalog(BaseSchemaCatalog):
if isinstance(change, FilesetChange.RemoveProperty):
return
FilesetUpdateRequest.RemoveFilesetPropertyRequest(change.property())
if isinstance(change, FilesetChange.RemoveComment):
- return FilesetUpdateRequest.RemoveFilesetCommentRequest()
+ return FilesetUpdateRequest.UpdateFilesetCommentRequest(None)
raise ValueError(f"Unknown change type: {type(change).__name__}")
diff --git
a/clients/client-python/gravitino/dto/requests/catalog_update_request.py
b/clients/client-python/gravitino/dto/requests/catalog_update_request.py
index 1164db299..1ea4d9953 100644
--- a/clients/client-python/gravitino/dto/requests/catalog_update_request.py
+++ b/clients/client-python/gravitino/dto/requests/catalog_update_request.py
@@ -78,8 +78,8 @@ class CatalogUpdateRequest:
return CatalogChange.update_comment(self._new_comment)
def validate(self):
- if not self._new_comment:
- raise ValueError('"newComment" field is required and cannot be
empty')
+ """Validates the fields of the request. Always pass."""
+ pass
@dataclass
class SetCatalogPropertyRequest(CatalogUpdateRequestBase):
diff --git
a/clients/client-python/gravitino/dto/requests/fileset_update_request.py
b/clients/client-python/gravitino/dto/requests/fileset_update_request.py
index 66e001bee..9a640d207 100644
--- a/clients/client-python/gravitino/dto/requests/fileset_update_request.py
+++ b/clients/client-python/gravitino/dto/requests/fileset_update_request.py
@@ -79,13 +79,8 @@ class FilesetUpdateRequest:
self._new_comment = new_comment
def validate(self):
- """Validates the fields of the request.
-
- Raises:
- IllegalArgumentException if the new comment is not set.
- """
- if not self._new_comment:
- raise ValueError('"new_comment" field is required and cannot
be empty')
+ """Validates the fields of the request. Always pass."""
+ pass
def fileset_change(self):
"""Returns the fileset change"""
@@ -149,7 +144,11 @@ class FilesetUpdateRequest:
@dataclass
class RemoveFilesetCommentRequest(FilesetUpdateRequestBase):
- """Represents a request to remove comment from a Fileset."""
+ """Represents a request to remove comment from a Fileset.
+
+ Deprecated:
+ Please use `UpdateFilesetCommentRequest` with null value as the
argument instead.
+ """
def __init__(self):
super().__init__("removeComment")
diff --git
a/clients/client-python/gravitino/dto/requests/metalake_update_request.py
b/clients/client-python/gravitino/dto/requests/metalake_update_request.py
index 8e54fe360..79551e5dc 100644
--- a/clients/client-python/gravitino/dto/requests/metalake_update_request.py
+++ b/clients/client-python/gravitino/dto/requests/metalake_update_request.py
@@ -74,13 +74,8 @@ class MetalakeUpdateRequest:
self._new_comment = new_comment
def validate(self):
- """Validates the fields of the request.
-
- Raises:
- IllegalArgumentException if the new comment is not set.
- """
- if not self._new_comment:
- raise ValueError('"newComment" field is required and cannot be
empty')
+ """Validates the fields of the request. Always pass."""
+ pass
def metalake_change(self):
return MetalakeChange.update_comment(self._new_comment)
diff --git a/clients/client-python/tests/integration/test_catalog.py
b/clients/client-python/tests/integration/test_catalog.py
index 64208315e..bd7933e00 100644
--- a/clients/client-python/tests/integration/test_catalog.py
+++ b/clients/client-python/tests/integration/test_catalog.py
@@ -125,6 +125,14 @@ class TestCatalog(IntegrationTestEnv):
with self.assertRaises(CatalogAlreadyExistsException):
_ = self.create_catalog(self.catalog_name)
+ def test_nullable_comment_catalog(self):
+ self.create_catalog(self.catalog_name)
+ changes = (CatalogChange.update_comment(None),)
+ null_comment_catalog = self.gravitino_client.alter_catalog(
+ self.catalog_name, *changes
+ )
+ self.assertIsNone(null_comment_catalog.comment())
+
def test_alter_catalog(self):
catalog = self.create_catalog(self.catalog_name)
diff --git a/clients/client-python/tests/integration/test_fileset_catalog.py
b/clients/client-python/tests/integration/test_fileset_catalog.py
index 754735b16..ac3d0a821 100644
--- a/clients/client-python/tests/integration/test_fileset_catalog.py
+++ b/clients/client-python/tests/integration/test_fileset_catalog.py
@@ -239,7 +239,7 @@ class TestFilesetCatalog(IntegrationTestEnv):
self.assertEqual(fileset_new.comment(), fileset_new_comment)
fileset_comment_removed = catalog.as_fileset_catalog().alter_fileset(
- self.fileset_ident, FilesetChange.remove_comment()
+ self.fileset_ident, FilesetChange.update_comment(None)
)
self.assertEqual(fileset_comment_removed.name(), self.fileset_name)
self.assertIsNone(fileset_comment_removed.comment())
diff --git a/clients/client-python/tests/integration/test_metalake.py
b/clients/client-python/tests/integration/test_metalake.py
index 75d3a06f2..f2b14b678 100644
--- a/clients/client-python/tests/integration/test_metalake.py
+++ b/clients/client-python/tests/integration/test_metalake.py
@@ -90,6 +90,14 @@ class TestMetalake(IntegrationTestEnv):
with self.assertRaises(MetalakeAlreadyExistsException):
_ = self.create_metalake(self.metalake_name)
+ def test_nullable_comment_metalake(self):
+ self.create_metalake(self.metalake_name)
+ changes = (MetalakeChange.update_comment(None),)
+ null_comment_metalake = self.gravitino_admin_client.alter_metalake(
+ self.metalake_name, *changes
+ )
+ self.assertIsNone(null_comment_metalake.comment())
+
def test_alter_metalake(self):
self.create_metalake(self.metalake_name)
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
index 9dfbb4efd..aabdf2901 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/CatalogUpdateRequest.java
@@ -117,17 +117,9 @@ public interface CatalogUpdateRequest extends RESTRequest {
this(null);
}
- /**
- * Validates the fields of the request.
- *
- * @throws IllegalArgumentException if the new comment is not set.
- */
+ /** Validates the fields of the request. Always pass. */
@Override
- public void validate() throws IllegalArgumentException {
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment),
- "\"newComment\" field is required and cannot be empty");
- }
+ public void validate() throws IllegalArgumentException {}
@Override
public CatalogChange catalogChange() {
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java
index c7aa79cb8..82915dcdc 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java
@@ -109,17 +109,9 @@ public interface FilesetUpdateRequest extends RESTRequest {
return FilesetChange.updateComment(newComment);
}
- /**
- * Validates the request.
- *
- * @throws IllegalArgumentException if the request is invalid.
- */
+ /** Validates the fields of the request. Always pass. */
@Override
- public void validate() throws IllegalArgumentException {
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment),
- "\"newComment\" field is required and cannot be empty");
- }
+ public void validate() throws IllegalArgumentException {}
}
/** The fileset update request for setting the properties of a fileset. */
@@ -194,7 +186,7 @@ public interface FilesetUpdateRequest extends RESTRequest {
/** @return The fileset change. */
@Override
public FilesetChange filesetChange() {
- return FilesetChange.removeComment();
+ return FilesetChange.updateComment(null);
}
/**
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java
index a5b747aff..6e01ace80 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/MetalakeUpdateRequest.java
@@ -117,17 +117,9 @@ public interface MetalakeUpdateRequest extends RESTRequest
{
this(null);
}
- /**
- * Validates the fields of the request.
- *
- * @throws IllegalArgumentException if the new comment is not set.
- */
+ /** Validates the fields of the request. Always pass. */
@Override
- public void validate() throws IllegalArgumentException {
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment),
- "\"newComment\" field is required and cannot be empty");
- }
+ public void validate() throws IllegalArgumentException {}
@Override
public MetalakeChange metalakeChange() {
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
index 936597f4d..db1702ce6 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/TableUpdateRequest.java
@@ -164,17 +164,9 @@ public interface TableUpdateRequest extends RESTRequest {
this(null);
}
- /**
- * Validates the request.
- *
- * @throws IllegalArgumentException If the request is invalid, this
exception is thrown.
- */
+ /** Validates the fields of the request. Always pass. */
@Override
- public void validate() throws IllegalArgumentException {
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment),
- "\"newComment\" field is required and cannot be empty");
- }
+ public void validate() throws IllegalArgumentException {}
/**
* Returns the table change.
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java
index 5323ac565..f27d8359e 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/TagUpdateRequest.java
@@ -114,11 +114,9 @@ public interface TagUpdateRequest extends RESTRequest {
return TagChange.updateComment(newComment);
}
+ /** Validates the fields of the request. Always pass. */
@Override
- public void validate() throws IllegalArgumentException {
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment), "\"newComment\" must not be
blank");
- }
+ public void validate() throws IllegalArgumentException {}
}
/** The tag update request for setting a tag property. */
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java
index 67103b2e5..add5ae7e9 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/TopicUpdateRequest.java
@@ -72,17 +72,9 @@ public interface TopicUpdateRequest extends RESTRequest {
this(null);
}
- /**
- * Validates the request.
- *
- * @throws IllegalArgumentException If the request is invalid, this
exception is thrown.
- */
+ /** Validates the fields of the request. Always pass. */
@Override
- public void validate() throws IllegalArgumentException {
- Preconditions.checkArgument(
- StringUtils.isNotBlank(newComment),
- "\"newComment\" field is required and cannot be empty");
- }
+ public void validate() throws IllegalArgumentException {}
/**
* Returns the topic change.
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
index 7ceed9e2e..4fa3cecbb 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestFilesetOperationDispatcher.java
@@ -143,7 +143,7 @@ public class TestFilesetOperationDispatcher extends
TestOperationDispatcher {
Assertions.assertEquals(fileset1.name(), alteredFileset2.name());
Assertions.assertEquals("new comment", alteredFileset2.comment());
- FilesetChange[] changes3 = new FilesetChange[]
{FilesetChange.removeComment()};
+ FilesetChange[] changes3 = new FilesetChange[]
{FilesetChange.updateComment(null)};
Fileset alteredFileset3 =
filesetOperationDispatcher.alterFileset(filesetIdent1, changes3);
Assertions.assertEquals(fileset1.name(), alteredFileset3.name());
diff --git a/docs/manage-fileset-metadata-using-gravitino.md
b/docs/manage-fileset-metadata-using-gravitino.md
index de478efb7..fe1d33040 100644
--- a/docs/manage-fileset-metadata-using-gravitino.md
+++ b/docs/manage-fileset-metadata-using-gravitino.md
@@ -389,13 +389,13 @@ fileset_new =
catalog.as_fileset_catalog().alter_fileset(NameIdentifier.of("sche
Currently, Gravitino supports the following changes to a fileset:
-| Supported modification | JSON
| Java |
-|----------------------------|--------------------------------------------------------------|-----------------------------------------------|
-| Rename a fileset |
`{"@type":"rename","newName":"fileset_renamed"}` |
`FilesetChange.rename("fileset_renamed")` |
-| Update a comment |
`{"@type":"updateComment","newComment":"new_comment"}` |
`FilesetChange.updateComment("new_comment")` |
-| Set a fileset property |
`{"@type":"setProperty","property":"key1","value":"value1"}` |
`FilesetChange.setProperty("key1", "value1")` |
-| Remove a fileset property | `{"@type":"removeProperty","property":"key1"}`
| `FilesetChange.removeProperty("key1")` |
-| Remove comment | `{"@type":"removeComment"}`
| `FilesetChange.removeComment()` |
+| Supported modification | JSON
| Java |
+|-----------------------------|--------------------------------------------------------------|-----------------------------------------------|
+| Rename a fileset |
`{"@type":"rename","newName":"fileset_renamed"}` |
`FilesetChange.rename("fileset_renamed")` |
+| Update a comment |
`{"@type":"updateComment","newComment":"new_comment"}` |
`FilesetChange.updateComment("new_comment")` |
+| Set a fileset property |
`{"@type":"setProperty","property":"key1","value":"value1"}` |
`FilesetChange.setProperty("key1", "value1")` |
+| Remove a fileset property | `{"@type":"removeProperty","property":"key1"}`
| `FilesetChange.removeProperty("key1")` |
+| Remove comment (deprecated) | `{"@type":"removeComment"}`
| `FilesetChange.removeComment()` |
### Drop a fileset
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
index 62375dc4b..4258346e4 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
@@ -370,7 +370,7 @@ public class TestFilesetOperations extends JerseyTest {
@Test
public void testRemoveFilesetComment() {
- FilesetUpdateRequest req = new
FilesetUpdateRequest.RemoveFilesetCommentRequest();
+ FilesetUpdateRequest req = new
FilesetUpdateRequest.UpdateFilesetCommentRequest(null);
Fileset fileset =
mockFileset("fileset1", Fileset.Type.MANAGED, null, "mock location",
ImmutableMap.of());
assertUpdateFileset(new FilesetUpdatesRequest(ImmutableList.of(req)),
fileset);
@@ -387,7 +387,7 @@ public class TestFilesetOperations extends JerseyTest {
// remove k2
FilesetUpdateRequest req5 = new
FilesetUpdateRequest.RemoveFilesetPropertiesRequest("k2");
// remove comment
- FilesetUpdateRequest req6 = new
FilesetUpdateRequest.RemoveFilesetCommentRequest();
+ FilesetUpdateRequest req6 = new
FilesetUpdateRequest.UpdateFilesetCommentRequest(null);
Fileset fileset =
mockFileset(
