This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 450b1788d8 [#6525] feature(core): Support update comment operations
for model alteration. (#7041)
450b1788d8 is described below
commit 450b1788d849a314199166efb824a52693e721d0
Author: Lord of Abyss <[email protected]>
AuthorDate: Wed Apr 23 14:08:34 2025 +0800
[#6525] feature(core): Support update comment operations for model
alteration. (#7041)
### What changes were proposed in this pull request?
Support update comment operations for model alteration.
### Why are the changes needed?
Fix: #6525
### Does this PR introduce _any_ user-facing change?
User can update a comment of a model.
### How was this patch tested?
local test + ut + it.
`bin/gcli.sh model update -m demo_metalake --name
model_catalog.schema.model2 --comment 'new_comment'`
`bin/gcli.sh model update -m demo_metalake --name
model_catalog.schema.model2 --comment ''`
---
.../org/apache/gravitino/model/ModelChange.java | 71 +++++++++++++++++
.../apache/gravitino/model/TestModelChange.java | 38 +++++++++
.../catalog/model/ModelCatalogOperations.java | 6 ++
.../catalog/model/TestModelCatalogOperations.java | 36 +++++++++
.../integration/test/ModelCatalogOperationsIT.java | 20 ++++-
.../apache/gravitino/cli/ModelCommandHandler.java | 9 +++
.../apache/gravitino/cli/TestableCommandLine.java | 11 +++
.../gravitino/cli/commands/UpdateModelComment.java | 90 ++++++++++++++++++++++
.../apache/gravitino/cli/TestModelCommands.java | 30 ++++++++
.../org/apache/gravitino/client/DTOConverters.java | 4 +
.../client-python/gravitino/api/model_change.py | 53 +++++++++++++
.../gravitino/client/generic_model_catalog.py | 3 +
.../gravitino/dto/requests/model_update_request.py | 18 +++++
.../tests/integration/test_model_catalog.py | 34 +++++++-
.../gravitino/dto/requests/ModelUpdateRequest.java | 28 ++++++-
.../catalog/TestModelOperationDispatcher.java | 33 +++++++-
.../gravitino/connector/TestCatalogOperations.java | 11 ++-
.../relational/service/TestModelMetaService.java | 89 ++++++++++++++++++++-
docs/manage-model-metadata-using-gravitino.md | 19 +++--
19 files changed, 588 insertions(+), 15 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/model/ModelChange.java
b/api/src/main/java/org/apache/gravitino/model/ModelChange.java
index 1534b9369a..e0622a2313 100644
--- a/api/src/main/java/org/apache/gravitino/model/ModelChange.java
+++ b/api/src/main/java/org/apache/gravitino/model/ModelChange.java
@@ -59,6 +59,16 @@ public interface ModelChange {
return new ModelChange.RemoveProperty(property);
}
+ /**
+ * Create a ModelChange for updating the comment of a model.
+ *
+ * @param newComment The new comment of the model.
+ * @return A ModelChange for the comment update.
+ */
+ static ModelChange updateComment(String newComment) {
+ return new ModelChange.UpdateComment(newComment);
+ }
+
/** A ModelChange to rename a model. */
final class RenameModel implements ModelChange {
private final String newName;
@@ -253,4 +263,65 @@ public interface ModelChange {
return "RemoveProperty " + property;
}
}
+
+ /** A ModelChange to update the comment of a model. */
+ final class UpdateComment implements ModelChange {
+ private final String newComment;
+
+ /**
+ * Constructs a new {@link UpdateComment} instance with the specified new
comment.
+ *
+ * @param newComment The new comment of the model.
+ */
+ public UpdateComment(String newComment) {
+ this.newComment = newComment;
+ }
+
+ /**
+ * Retrieves the new comment for the model.
+ *
+ * @return The new comment of the model.
+ */
+ public String newComment() {
+ return newComment;
+ }
+
+ /**
+ * Compares this UpdateComment instance with another object for equality.
The comparison is
+ * based on the new comment of the model.
+ *
+ * @param obj The object to compare with this instance.
+ * @return {@code true} if the given object represents the same model
comment update; {@code
+ * false} otherwise.
+ */
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == this) return true;
+ if (!(obj instanceof UpdateComment)) return false;
+ UpdateComment other = (UpdateComment) obj;
+ return Objects.equals(newComment, other.newComment);
+ }
+
+ /**
+ * Generates a hash code for this UpdateComment instance. The hash code is
based on the new
+ * comment of the model.
+ *
+ * @return A hash code value for this model comment update operation.
+ */
+ @Override
+ public int hashCode() {
+ return Objects.hash(newComment);
+ }
+
+ /**
+ * Provides a string representation of the UpdateComment instance. This
string format includes
+ * the class name followed by the new comment of the model.
+ *
+ * @return A string summary of the model comment update operation.
+ */
+ @Override
+ public String toString() {
+ return "UpdateComment " + newComment;
+ }
+ }
}
diff --git a/api/src/test/java/org/apache/gravitino/model/TestModelChange.java
b/api/src/test/java/org/apache/gravitino/model/TestModelChange.java
index a906f8a75b..f79fbc9fe2 100644
--- a/api/src/test/java/org/apache/gravitino/model/TestModelChange.java
+++ b/api/src/test/java/org/apache/gravitino/model/TestModelChange.java
@@ -60,4 +60,42 @@ public class TestModelChange {
Assertions.assertNotEquals(renameModel1.hashCode(),
renameModel2.hashCode());
Assertions.assertEquals(renameModel1.hashCode(), renameModel3.hashCode());
}
+
+ @Test
+ void testUpdateModelCommentWithStaticMethod() {
+ String newComment = "newComment";
+ ModelChange modelChange = ModelChange.updateComment(newComment);
+ Assertions.assertEquals(ModelChange.UpdateComment.class,
modelChange.getClass());
+
+ ModelChange.UpdateComment updateComment = (ModelChange.UpdateComment)
modelChange;
+ Assertions.assertEquals(newComment, updateComment.newComment());
+ Assertions.assertEquals("UpdateComment newComment",
updateComment.toString());
+ }
+
+ @Test
+ void testUpdateModelCommentWithConstructor() {
+ String newComment = "newComment";
+ ModelChange.UpdateComment updateComment = new
ModelChange.UpdateComment(newComment);
+ Assertions.assertEquals(newComment, updateComment.newComment());
+ Assertions.assertEquals("UpdateComment newComment",
updateComment.toString());
+ }
+
+ @Test
+ void testUpdateModelCommentEquals() {
+ String newComment1 = "This is a demo model";
+ String newComment2 = "This is a test model";
+ String newComment3 = "This is a demo model";
+
+ ModelChange.UpdateComment updateComment1 = new
ModelChange.UpdateComment(newComment1);
+ ModelChange.UpdateComment updateComment2 = new
ModelChange.UpdateComment(newComment2);
+ ModelChange.UpdateComment updateComment3 = new
ModelChange.UpdateComment(newComment3);
+
+ Assertions.assertEquals(updateComment1, updateComment3);
+ Assertions.assertNotEquals(updateComment1, updateComment2);
+ Assertions.assertEquals(updateComment1, updateComment3);
+
+ Assertions.assertNotEquals(updateComment1.hashCode(),
updateComment2.hashCode());
+ Assertions.assertEquals(updateComment1.hashCode(),
updateComment3.hashCode());
+ Assertions.assertEquals(updateComment1.toString(),
updateComment3.toString());
+ }
}
diff --git
a/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
b/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
index 87b0d12d45..47b709d957 100644
---
a/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
+++
b/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
@@ -334,12 +334,18 @@ public class ModelCatalogOperations extends
ManagedSchemaOperations
for (ModelChange change : changes) {
if (change instanceof ModelChange.RenameModel) {
entityName = ((ModelChange.RenameModel) change).newName();
+
} else if (change instanceof ModelChange.SetProperty) {
ModelChange.SetProperty setPropertyChange = (ModelChange.SetProperty)
change;
doSetProperty(entityProperties, setPropertyChange);
+
} else if (change instanceof ModelChange.RemoveProperty) {
ModelChange.RemoveProperty removePropertyChange =
(ModelChange.RemoveProperty) change;
doRemoveProperty(entityProperties, removePropertyChange);
+
+ } else if (change instanceof ModelChange.UpdateComment) {
+ entityComment = ((ModelChange.UpdateComment) change).newComment();
+
} else {
throw new IllegalArgumentException(
"Unsupported model change: " + change.getClass().getSimpleName());
diff --git
a/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
b/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
index 77331cf673..882b1ec798 100644
---
a/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
+++
b/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
@@ -800,6 +800,42 @@ public class TestModelCatalogOperations {
Assertions.assertEquals(newProperties, alteredModel.properties());
}
+ @Test
+ void testUpdateModelComment() {
+ String schemaName = randomSchemaName();
+ createSchema(schemaName);
+
+ String modelName = "model";
+ String comment = "comment";
+ String newComment = "new comment";
+
+ NameIdentifier modelIdent =
+ NameIdentifierUtil.ofModel(METALAKE_NAME, CATALOG_NAME, schemaName,
modelName);
+ StringIdentifier stringId = StringIdentifier.fromId(idGenerator.nextId());
+ Map<String, String> properties =
+ StringIdentifier.newPropertiesWithId(stringId, ImmutableMap.of("key1",
"value1"));
+
+ // validate registered model
+ Model registeredModel = ops.registerModel(modelIdent, comment, properties);
+ Assertions.assertEquals(modelName, registeredModel.name());
+ Assertions.assertEquals(comment, registeredModel.comment());
+ Assertions.assertEquals(properties, registeredModel.properties());
+
+ // validate loaded model
+ Model loadedModel = ops.getModel(modelIdent);
+ Assertions.assertEquals(modelName, loadedModel.name());
+ Assertions.assertEquals(comment, loadedModel.comment());
+ Assertions.assertEquals(properties, loadedModel.properties());
+
+ ModelChange change = ModelChange.updateComment(newComment);
+ Model alteredModel = ops.alterModel(modelIdent, change);
+
+ // validate altered model
+ Assertions.assertEquals(modelName, alteredModel.name());
+ Assertions.assertEquals(newComment, alteredModel.comment());
+ Assertions.assertEquals(properties, alteredModel.properties());
+ }
+
@Test
void testUpdateVersionCommentViaVersion() {
String schemaName = randomSchemaName();
diff --git
a/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/integration/test/ModelCatalogOperationsIT.java
b/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/integration/test/ModelCatalogOperationsIT.java
index 8590878d6d..ecda97cbad 100644
---
a/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/integration/test/ModelCatalogOperationsIT.java
+++
b/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/integration/test/ModelCatalogOperationsIT.java
@@ -395,7 +395,7 @@ public class ModelCatalogOperationsIT extends BaseIT {
}
@Test
- public void testRegisterAndUpdateModel() {
+ public void testRegisterAndRenameModel() {
String comment = "comment";
String modelName = RandomNameUtils.genRandomName("alter_name_model");
String newName = RandomNameUtils.genRandomName("new_name");
@@ -482,6 +482,24 @@ public class ModelCatalogOperationsIT extends BaseIT {
Assertions.assertEquals(createdModel.comment(), alteredModel.comment());
}
+ @Test
+ void testRegisterAndUpdateModelComment() {
+ String comment = "comment";
+ String newComment = "new comment";
+ String modelName = RandomNameUtils.genRandomName("alter_name_model");
+ NameIdentifier modelIdent = NameIdentifier.of(schemaName, modelName);
+ Map<String, String> properties = ImmutableMap.of("owner", "data-team",
"key1", "val1");
+
+ Model createdModel =
+ gravitinoCatalog.asModelCatalog().registerModel(modelIdent, comment,
properties);
+ ModelChange change = ModelChange.updateComment(newComment);
+ Model alteredModel =
gravitinoCatalog.asModelCatalog().alterModel(modelIdent, change);
+
+ Assertions.assertEquals(createdModel.name(), alteredModel.name());
+ Assertions.assertEquals(createdModel.properties(),
alteredModel.properties());
+ Assertions.assertEquals(newComment, alteredModel.comment());
+ }
+
@Test
void testLinkAndSetModelVersionProperties() {
String modelName = RandomNameUtils.genRandomName("model1");
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/ModelCommandHandler.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/ModelCommandHandler.java
index ee15d3a88f..7dec8bfb05 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/ModelCommandHandler.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/ModelCommandHandler.java
@@ -181,6 +181,15 @@ public class ModelCommandHandler extends CommandHandler {
.handle();
}
+ if (line.hasOption(GravitinoOptions.COMMENT)
+ && !(line.hasOption(GravitinoOptions.ALIAS) ||
line.hasOption(GravitinoOptions.VERSION))) {
+ String newComment = line.getOptionValue(GravitinoOptions.COMMENT);
+ gravitinoCommandLine
+ .newUpdateModelComment(context, metalake, catalog, schema, model,
newComment)
+ .validate()
+ .handle();
+ }
+
if (!line.hasOption(GravitinoOptions.URI)
&& line.hasOption(GravitinoOptions.COMMENT)
&& (line.hasOption(GravitinoOptions.ALIAS) ||
line.hasOption(GravitinoOptions.VERSION))) {
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
index b32a758ef6..81453fc305 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
@@ -136,6 +136,7 @@ import
org.apache.gravitino.cli.commands.UpdateFilesetComment;
import org.apache.gravitino.cli.commands.UpdateFilesetName;
import org.apache.gravitino.cli.commands.UpdateMetalakeComment;
import org.apache.gravitino.cli.commands.UpdateMetalakeName;
+import org.apache.gravitino.cli.commands.UpdateModelComment;
import org.apache.gravitino.cli.commands.UpdateModelName;
import org.apache.gravitino.cli.commands.UpdateModelVersionComment;
import org.apache.gravitino.cli.commands.UpdateModelVersionUri;
@@ -889,6 +890,16 @@ public class TestableCommandLine {
return new UpdateModelName(context, metalake, catalog, schema, model,
rename);
}
+ protected UpdateModelComment newUpdateModelComment(
+ CommandContext context,
+ String metalake,
+ String catalog,
+ String schema,
+ String model,
+ String comment) {
+ return new UpdateModelComment(context, metalake, catalog, schema, model,
comment);
+ }
+
protected UpdateModelVersionComment newUpdateModelVersionComment(
CommandContext context,
String metalake,
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UpdateModelComment.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UpdateModelComment.java
new file mode 100644
index 0000000000..1e34189f97
--- /dev/null
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UpdateModelComment.java
@@ -0,0 +1,90 @@
+/*
+ * 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.
+ */
+
+package org.apache.gravitino.cli.commands;
+
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.cli.CommandContext;
+import org.apache.gravitino.cli.ErrorMessages;
+import org.apache.gravitino.client.GravitinoClient;
+import org.apache.gravitino.exceptions.NoSuchCatalogException;
+import org.apache.gravitino.exceptions.NoSuchMetalakeException;
+import org.apache.gravitino.exceptions.NoSuchModelException;
+import org.apache.gravitino.exceptions.NoSuchSchemaException;
+import org.apache.gravitino.model.ModelChange;
+
+/** Update the comment of a model. */
+public class UpdateModelComment extends Command {
+ protected final String metalake;
+ protected final String catalog;
+ protected final String schema;
+ protected final String model;
+ protected final String comment;
+
+ /**
+ * Construct a new {@link UpdateModelComment} instance.
+ *
+ * @param context The command context.
+ * @param metalake The metalake name.
+ * @param catalog The catalog name.
+ * @param schema The schema name.
+ * @param model The model name.
+ * @param comment The new comment.
+ */
+ public UpdateModelComment(
+ CommandContext context,
+ String metalake,
+ String catalog,
+ String schema,
+ String model,
+ String comment) {
+ super(context);
+ this.metalake = metalake;
+ this.catalog = catalog;
+ this.schema = schema;
+ this.model = model;
+ this.comment = comment;
+ }
+
+ /** Update the comment of a model. */
+ @Override
+ public void handle() {
+ NameIdentifier modelIdent;
+
+ try {
+ modelIdent = NameIdentifier.of(schema, model);
+ GravitinoClient client = buildClient(metalake);
+ ModelChange updateCommentChange = ModelChange.updateComment(comment);
+
+ client.loadCatalog(catalog).asModelCatalog().alterModel(modelIdent,
updateCommentChange);
+ } catch (NoSuchMetalakeException err) {
+ exitWithError(ErrorMessages.UNKNOWN_METALAKE);
+ } catch (NoSuchCatalogException err) {
+ exitWithError(ErrorMessages.UNKNOWN_CATALOG);
+ } catch (NoSuchSchemaException err) {
+ exitWithError(ErrorMessages.UNKNOWN_SCHEMA);
+ } catch (NoSuchModelException err) {
+ exitWithError(ErrorMessages.UNKNOWN_MODEL);
+ } catch (Exception exp) {
+ exitWithError(exp.getMessage());
+ }
+
+ printInformation(model + " comment changed.");
+ }
+}
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestModelCommands.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestModelCommands.java
index 99b17bce23..7919e85200 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestModelCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestModelCommands.java
@@ -50,6 +50,7 @@ import org.apache.gravitino.cli.commands.RemoveModelProperty;
import org.apache.gravitino.cli.commands.RemoveModelVersionProperty;
import org.apache.gravitino.cli.commands.SetModelProperty;
import org.apache.gravitino.cli.commands.SetModelVersionProperty;
+import org.apache.gravitino.cli.commands.UpdateModelComment;
import org.apache.gravitino.cli.commands.UpdateModelName;
import org.apache.gravitino.cli.commands.UpdateModelVersionComment;
import org.apache.gravitino.cli.commands.UpdateModelVersionUri;
@@ -667,6 +668,35 @@ public class TestModelCommands {
verify(mockRemoveProperty).handle();
}
+ @Test
+ void testUpdateModelComment() {
+ UpdateModelComment mockUpdate = mock(UpdateModelComment.class);
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog.schema.model");
+ when(mockCommandLine.hasOption(GravitinoOptions.COMMENT)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.COMMENT)).thenReturn("new
comment");
+
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.MODEL,
CommandActions.UPDATE));
+
+ doReturn(mockUpdate)
+ .when(commandLine)
+ .newUpdateModelComment(
+ any(CommandContext.class),
+ eq("metalake_demo"),
+ eq("catalog"),
+ eq("schema"),
+ eq("model"),
+ eq("new comment"));
+ doReturn(mockUpdate).when(mockUpdate).validate();
+ commandLine.handleCommandLine();
+ verify(mockUpdate).handle();
+ }
+
@Test
void testUpdateModelVersionCommentCommand() {
UpdateModelVersionComment mockUpdate =
mock(UpdateModelVersionComment.class);
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
index f683f1ad94..f911d4874e 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
@@ -374,6 +374,10 @@ class DTOConverters {
((ModelChange.SetProperty) change).property(),
((ModelChange.SetProperty) change).value());
+ } else if (change instanceof ModelChange.UpdateComment) {
+ return new ModelUpdateRequest.UpdateModelCommentRequest(
+ ((ModelChange.UpdateComment) change).newComment());
+
} else {
throw new IllegalArgumentException(
"Unknown model change type: " + change.getClass().getSimpleName());
diff --git a/clients/client-python/gravitino/api/model_change.py
b/clients/client-python/gravitino/api/model_change.py
index 99737d7823..16041cfe17 100644
--- a/clients/client-python/gravitino/api/model_change.py
+++ b/clients/client-python/gravitino/api/model_change.py
@@ -55,6 +55,16 @@ class ModelChange(ABC):
"""
return ModelChange.RemoveProperty(pro)
+ @staticmethod
+ def update_comment(comment):
+ """Creates a new model change to update the comment of the model.
+ Args:
+ comment: The new comment of the model.
+ Returns:
+ The model change.
+ """
+ return ModelChange.UpdateComment(comment)
+
class RenameModel:
"""A model change to rename the model."""
@@ -193,3 +203,46 @@ class ModelChange(ABC):
A string summary of this property removal operation.
"""
return f"REMOVEPROPERTY {self.property()}"
+
+ class UpdateComment:
+ """
+ A model change to update the comment of the model.
+ """
+
+ def __init__(self, new_comment):
+ self._new_comment = new_comment
+
+ def new_comment(self):
+ """Retrieves the comment of the model.
+ Returns:
+ The comment of the model.
+ """
+ return self._new_comment
+
+ def __eq__(self, other) -> bool:
+ """Compares this UpdateComment instance with another object for
equality. Two instances are
+ considered equal if they designate the same comment for the model.
+ Args:
+ other: The object to compare with this instance.
+ Returns:
+ true if the given object represents an identical model comment
update operation; false otherwise.
+ """
+ if not isinstance(other, ModelChange.UpdateComment):
+ return False
+ return self.new_comment() == other.new_comment()
+
+ def __hash__(self):
+ """Generates a hash code for this UpdateComment instance. The hash
code is primarily based on
+ the comment for the model.
+ Returns:
+ A hash code value for this comment update operation.
+ """
+ return hash(self.new_comment())
+
+ def __str__(self):
+ """Provides a string representation of the UpdateComment instance.
This string includes the
+ class name followed by the comment of the model.
+ Returns:
+ A string summary of this comment update operation.
+ """
+ return f"UpdateComment {self.new_comment()}"
diff --git a/clients/client-python/gravitino/client/generic_model_catalog.py
b/clients/client-python/gravitino/client/generic_model_catalog.py
index 21e63b755a..318748781a 100644
--- a/clients/client-python/gravitino/client/generic_model_catalog.py
+++ b/clients/client-python/gravitino/client/generic_model_catalog.py
@@ -523,6 +523,9 @@ class GenericModelCatalog(BaseSchemaCatalog):
if isinstance(change, ModelChange.RemoveProperty):
return
ModelUpdateRequest.ModelRemovePropertyRequest(change.property())
+ if isinstance(change, ModelChange.UpdateComment):
+ return
ModelUpdateRequest.UpdateModelCommentRequest(change.new_comment())
+
raise ValueError(f"Unknown change type: {type(change).__name__}")
@staticmethod
diff --git
a/clients/client-python/gravitino/dto/requests/model_update_request.py
b/clients/client-python/gravitino/dto/requests/model_update_request.py
index 5769f2015f..7e4fed0863 100644
--- a/clients/client-python/gravitino/dto/requests/model_update_request.py
+++ b/clients/client-python/gravitino/dto/requests/model_update_request.py
@@ -97,3 +97,21 @@ class ModelUpdateRequest:
def model_change(self) -> ModelChange:
return ModelChange.remove_property(self._property)
+
+ @dataclass
+ class UpdateModelCommentRequest(ModelUpdateRequestBase):
+ """Request to update model comment"""
+
+ _new_comment: Optional[str] =
field(metadata=config(field_name="newComment"))
+ """Represents a request to update the comment on a Metalake."""
+
+ def __init__(self, new_comment: str):
+ super().__init__("updateComment")
+ self._new_comment = new_comment
+
+ def validate(self):
+ """Validates the fields of the request. Always pass."""
+ pass
+
+ def model_change(self):
+ return ModelChange.update_comment(self._new_comment)
diff --git a/clients/client-python/tests/integration/test_model_catalog.py
b/clients/client-python/tests/integration/test_model_catalog.py
index 3ef9ea8b08..5d0eb9218a 100644
--- a/clients/client-python/tests/integration/test_model_catalog.py
+++ b/clients/client-python/tests/integration/test_model_catalog.py
@@ -201,7 +201,7 @@ class TestModelCatalog(IntegrationTestEnv):
)
)
- def test_register_alter_model(self):
+ def test_register_rename_model(self):
model_name = f"model_it_model{str(randint(0, 1000))}"
model_new_name = f"model_it_model_new{str(randint(0, 1000))}"
model_ident = NameIdentifier.of(self._schema_name, model_name)
@@ -228,7 +228,7 @@ class TestModelCatalog(IntegrationTestEnv):
self.assertEqual(0, renamed_model.latest_version())
self.assertEqual(properties, renamed_model.properties())
- def test_register_alter_model_with_set_property(self):
+ def test_register_set_model_property(self):
model_name = f"model_it_model{str(randint(0, 1000))}"
model_ident = NameIdentifier.of(self._schema_name, model_name)
comment = "comment"
@@ -258,7 +258,7 @@ class TestModelCatalog(IntegrationTestEnv):
update_property_model.properties(), {"k1": "v11", "k2": "v2",
"k3": "v3"}
)
- def test_register_alter_model_with_remove_property(self):
+ def test_register_remove_model_property(self):
model_name = f"model_it_model{str(randint(0, 1000))}"
model_ident = NameIdentifier.of(self._schema_name, model_name)
comment = "comment"
@@ -281,7 +281,33 @@ class TestModelCatalog(IntegrationTestEnv):
self.assertEqual(update_property_model.latest_version(), 0)
self.assertEqual(update_property_model.properties(), {"k2": "v2"})
- def test_register_alter_model_with_update_comment(self):
+ def test_register_update_model_comment(self):
+ model_name = f"model_it_model{str(randint(0, 1000))}"
+ model_ident = NameIdentifier.of(self._schema_name, model_name)
+ comment = "comment"
+ new_comment = "new comment"
+ properties = {"k1": "v1", "k2": "v2"}
+ self._catalog.as_model_catalog().register_model(
+ model_ident, comment, properties
+ )
+
+ # Retrieve the original model
+ origin_model = self._catalog.as_model_catalog().get_model(model_ident)
+ self.assertEqual(origin_model.name(), model_name)
+ self.assertEqual(origin_model.comment(), comment)
+ self.assertEqual(origin_model.latest_version(), 0)
+ self.assertEqual(origin_model.properties(), properties)
+
+ # Alter model and validate the updated model
+ changes = [ModelChange.update_comment(new_comment)]
+ self._catalog.as_model_catalog().alter_model(model_ident, *changes)
+ update_comment_model =
self._catalog.as_model_catalog().get_model(model_ident)
+ self.assertEqual(update_comment_model.name(), model_name)
+ self.assertEqual(update_comment_model.comment(), new_comment)
+ self.assertEqual(update_comment_model.latest_version(), 0)
+ self.assertEqual(update_comment_model.properties(), properties)
+
+ def test_link_update_model_version_comment(self):
model_name = f"model_it_model{str(randint(0, 1000))}"
model_ident = NameIdentifier.of(self._schema_name, model_name)
self._catalog.as_model_catalog().register_model(model_ident,
"comment", {})
diff --git
a/common/src/main/java/org/apache/gravitino/dto/requests/ModelUpdateRequest.java
b/common/src/main/java/org/apache/gravitino/dto/requests/ModelUpdateRequest.java
index ad06d75597..12459d2c1a 100644
---
a/common/src/main/java/org/apache/gravitino/dto/requests/ModelUpdateRequest.java
+++
b/common/src/main/java/org/apache/gravitino/dto/requests/ModelUpdateRequest.java
@@ -41,7 +41,12 @@ import org.apache.gravitino.rest.RESTRequest;
@JsonSubTypes.Type(
value = ModelUpdateRequest.RemoveModelPropertyRequest.class,
name = "removeProperty"),
- @JsonSubTypes.Type(value = ModelUpdateRequest.SetModelPropertyRequest.class,
name = "setProperty")
+ @JsonSubTypes.Type(
+ value = ModelUpdateRequest.SetModelPropertyRequest.class,
+ name = "setProperty"),
+ @JsonSubTypes.Type(
+ value = ModelUpdateRequest.UpdateModelCommentRequest.class,
+ name = "updateComment")
})
public interface ModelUpdateRequest extends RESTRequest {
@@ -157,4 +162,25 @@ public interface ModelUpdateRequest extends RESTRequest {
StringUtils.isNotBlank(property), "\"property\" field is required
and cannot be empty");
}
}
+
+ /** The model update request for update comment of model. */
+ @EqualsAndHashCode
+ @AllArgsConstructor
+ @NoArgsConstructor(force = true)
+ @ToString
+ @Getter
+ class UpdateModelCommentRequest implements ModelUpdateRequest {
+ @JsonProperty("newComment")
+ private final String newComment;
+
+ /** {@inheritDoc} */
+ @Override
+ public ModelChange modelChange() {
+ return ModelChange.updateComment(newComment);
+ }
+
+ /** Validates the fields of the request. Always pass. */
+ @Override
+ public void validate() throws IllegalArgumentException {}
+ }
}
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestModelOperationDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestModelOperationDispatcher.java
index f059b857c1..eb086a4de3 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestModelOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestModelOperationDispatcher.java
@@ -339,7 +339,7 @@ public class TestModelOperationDispatcher extends
TestOperationDispatcher {
@Test
void testRemoveModelProperty() {
String schemaName = "test_remove_model_property_schema";
- String modelName = "test_update_model_property";
+ String modelName = "test_remove_model_property";
String modelComment = "model which tests update property";
NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalog,
schemaName);
schemaOperationDispatcher.createSchema(
@@ -364,6 +364,37 @@ public class TestModelOperationDispatcher extends
TestOperationDispatcher {
Assertions.assertEquals(ImmutableMap.of("k2", "v2"),
alteredModel.properties());
}
+ @Test
+ void testUpdateModelComment() {
+ String schemaName = "test_update_model_comment_schema";
+ String modelName = "test_update_model_comment";
+ String modelComment = "model which tests update property";
+ String newModelComment = "new model comment";
+ Map<String, String> props = ImmutableMap.of("k1", "v1", "k2", "v2");
+
+ NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalog,
schemaName);
+ schemaOperationDispatcher.createSchema(
+ schemaIdent, "schema comment", ImmutableMap.of("k1", "v1", "k2",
"v2"));
+
+ NameIdentifier modelIdent =
+ NameIdentifierUtil.ofModel(metalake, catalog, schemaName, modelName);
+
+ Model model = modelOperationDispatcher.registerModel(modelIdent,
modelComment, props);
+
+ // validate registered model
+ Assertions.assertEquals(modelName, model.name());
+ Assertions.assertEquals(modelComment, model.comment());
+ Assertions.assertEquals(props, model.properties());
+
+ ModelChange change = ModelChange.updateComment(newModelComment);
+ Model alteredModel = modelOperationDispatcher.alterModel(modelIdent,
change);
+
+ // validate updated model
+ Assertions.assertEquals(modelName, alteredModel.name());
+ Assertions.assertEquals(newModelComment, alteredModel.comment());
+ Assertions.assertEquals(props, alteredModel.properties());
+ }
+
@Test
void testUpdateModelVersionComment() {
String schemaName = randomSchemaName();
diff --git
a/core/src/test/java/org/apache/gravitino/connector/TestCatalogOperations.java
b/core/src/test/java/org/apache/gravitino/connector/TestCatalogOperations.java
index a0105e0878..8e91f21575 100644
---
a/core/src/test/java/org/apache/gravitino/connector/TestCatalogOperations.java
+++
b/core/src/test/java/org/apache/gravitino/connector/TestCatalogOperations.java
@@ -956,12 +956,21 @@ public class TestCatalogOperations
if (models.containsKey(newIdent)) {
throw new ModelAlreadyExistsException("Model %s already exists",
ident);
}
+
} else if (change instanceof ModelChange.RemoveProperty) {
ModelChange.RemoveProperty removeProperty =
(ModelChange.RemoveProperty) change;
newProps.remove(removeProperty.property());
+
} else if (change instanceof ModelChange.SetProperty) {
ModelChange.SetProperty setProperty = (ModelChange.SetProperty) change;
newProps.put(setProperty.property(), setProperty.value());
+
+ } else if (change instanceof ModelChange.UpdateComment) {
+ ModelChange.UpdateComment updateComment = (ModelChange.UpdateComment)
change;
+ newComment = updateComment.newComment();
+
+ } else {
+ throw new IllegalArgumentException("Unsupported model change: " +
change);
}
}
TestModel updatedModel =
@@ -1060,7 +1069,7 @@ public class TestCatalogOperations
newUri = updateUriChange.newUri();
} else {
- throw new IllegalArgumentException("Unsupported model change: " +
change);
+ throw new IllegalArgumentException("Unsupported model version change:
" + change);
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelMetaService.java
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelMetaService.java
index 5f4ac8c3cb..b66601aae9 100644
---
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelMetaService.java
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelMetaService.java
@@ -202,7 +202,7 @@ public class TestModelMetaService extends TestJDBCBackend {
}
@Test
- void testAlterModel() throws IOException {
+ void testInsertAndRenameModel() throws IOException {
createParentEntities(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, auditInfo);
Map<String, String> properties = ImmutableMap.of("k1", "v1");
String newName = "new_model_name";
@@ -228,6 +228,93 @@ public class TestModelMetaService extends TestJDBCBackend {
.withLatestVersion(modelEntity.latestVersion())
.withAuditInfo(modelEntity.auditInfo())
.withComment(modelEntity.comment())
+ .withProperties(modelEntity.properties())
+ .build();
+
+ Function<ModelEntity, ModelEntity> renameUpdater = oldModel ->
updatedModel;
+ ModelEntity alteredModel =
+
ModelMetaService.getInstance().updateModel(modelEntity.nameIdentifier(),
renameUpdater);
+
+ Assertions.assertEquals(alteredModel, updatedModel);
+ // Test update an in-existent model
+ Assertions.assertThrows(
+ NoSuchEntityException.class,
+ () ->
+ ModelMetaService.getInstance()
+ .updateModel(NameIdentifier.of(MODEL_NS, "model3"),
renameUpdater));
+ }
+
+ @Test
+ void testInsertAndUpdateModelComment() throws IOException {
+ createParentEntities(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, auditInfo);
+ Map<String, String> properties = ImmutableMap.of("k1", "v1");
+ String newComment = "new_model_comment";
+
+ ModelEntity modelEntity =
+ createModelEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ MODEL_NS,
+ "model1",
+ "model1 comment",
+ 0,
+ properties,
+ auditInfo);
+
+ Assertions.assertDoesNotThrow(
+ () -> ModelMetaService.getInstance().insertModel(modelEntity, false));
+
+ ModelEntity updatedModel =
+ ModelEntity.builder()
+ .withId(modelEntity.id())
+ .withName(newComment)
+ .withNamespace(modelEntity.namespace())
+ .withLatestVersion(modelEntity.latestVersion())
+ .withAuditInfo(modelEntity.auditInfo())
+ .withComment(modelEntity.comment())
+ .withProperties(modelEntity.properties())
+ .build();
+
+ Function<ModelEntity, ModelEntity> renameUpdater = oldModel ->
updatedModel;
+ ModelEntity alteredModel =
+
ModelMetaService.getInstance().updateModel(modelEntity.nameIdentifier(),
renameUpdater);
+
+ Assertions.assertEquals(alteredModel, updatedModel);
+ // Test update an in-existent model
+ Assertions.assertThrows(
+ NoSuchEntityException.class,
+ () ->
+ ModelMetaService.getInstance()
+ .updateModel(NameIdentifier.of(MODEL_NS, "model3"),
renameUpdater));
+ }
+
+ @Test
+ void testInsertAndUpdateModelProperties() throws IOException {
+ createParentEntities(METALAKE_NAME, CATALOG_NAME, SCHEMA_NAME, auditInfo);
+ Map<String, String> properties = ImmutableMap.of("k1", "v1", "k2", "v2");
+ Map<String, String> newProps = ImmutableMap.of("k1", "v1", "k3", "v3");
+
+ ModelEntity modelEntity =
+ createModelEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ MODEL_NS,
+ "model1",
+ "model1 comment",
+ 0,
+ properties,
+ auditInfo);
+
+ Assertions.assertDoesNotThrow(
+ () -> ModelMetaService.getInstance().insertModel(modelEntity, false));
+
+ ModelEntity updatedModel =
+ ModelEntity.builder()
+ .withId(modelEntity.id())
+ .withName(modelEntity.comment())
+ .withNamespace(modelEntity.namespace())
+ .withLatestVersion(modelEntity.latestVersion())
+ .withAuditInfo(modelEntity.auditInfo())
+ .withComment(modelEntity.comment())
+ .withProperties(newProps)
.build();
Function<ModelEntity, ModelEntity> renameUpdater = oldModel ->
updatedModel;
diff --git a/docs/manage-model-metadata-using-gravitino.md
b/docs/manage-model-metadata-using-gravitino.md
index d0b8143cb7..46893fa443 100644
--- a/docs/manage-model-metadata-using-gravitino.md
+++ b/docs/manage-model-metadata-using-gravitino.md
@@ -304,7 +304,7 @@ model: Model =
catalog.as_model_catalog().get_model(ident=NameIdentifier.of("mod
### Alter a model
-You can modify a model's metadata (e.g., rename or modify properties) by
+You can modify a model's metadata (e.g. rename, update comment, or modify
properties) by
sending a `PUT` request to the
`/api/metalakes/{metalake_name}/catalogs/{catalog_name}/schemas/
{schema_name}/models/{model_name}` endpoint or using the Gravitino Java/Python
client. The following is an example of modifying a model:
@@ -315,6 +315,10 @@ sending a `PUT` request to the
`/api/metalakes/{metalake_name}/catalogs/{catalog
cat <<EOF >model.json
{
"updates": [
+ {
+ "@type": "updateComment",
+ "newComment": "Updated model comment"
+ },
{
"@type": "rename",
"newName": "new_name"
@@ -356,6 +360,7 @@ curl -X PUT \
// Define modifications
ModelChange[] changes = {
ModelChange.rename("example_model_renamed"),
+ ModelChange.updatComment("new comment"),
ModelChange.setProperty("k2", "v2"),
ModelChange.removeProperty("k1")
};
@@ -379,6 +384,7 @@ catalog =
client.load_catalog(name="mycatalog").as_model_catalog()
# Define modifications
changes = (
ModelChange.rename("renamed"),
+ ModelChange.update_comment("new comment"),
ModelChange.set_property("k2", "v2"),
ModelChange.remove_property("k1"),
)
@@ -396,11 +402,12 @@ updated_model = model_catalog.alter_model(
The following operations are supported for altering a model:
-| Operation | JSON Example
| Java Method | Python Method
|
-
|---------------------|------------------------------------------------------------|-------------------------------------------|--------------------------------------------|
-| **Rename model** | `{"@type":"rename","newName":"new_name"}`
| `ModelChange.rename("new_name")` |
`ModelChange.rename("new_name")` |
-| **Set property** |
`{"@type":"setProperty","property":"key","value":"value"}` |
`ModelChange.setProperty("key", "value")` | `ModelChange.set_property("key",
"value")` |
-| **Remove property** | `{"@type":"removeProperty","property":"key"}`
| `ModelChange.removeProperty("key")` |
`ModelChange.remove_property("key")` |
+| Operation | JSON Example
| Java Method | Python Method
|
+
|---------------------|------------------------------------------------------------|--------------------------------------------|---------------------------------------------|
+| **Rename model** | `{"@type":"rename","newName":"new_name"}`
| `ModelChange.rename("new_name")` |
`ModelChange.rename("new_name")` |
+| **Update comment** | `{"@type":"updateComment","newComment":"new comment"}`
| `ModelChange.updateComment("new comment")` |
`ModelChange.update_comment("new comment")` |
+| **Set property** |
`{"@type":"setProperty","property":"key","value":"value"}` |
`ModelChange.setProperty("key", "value")` | `ModelChange.set_property("key",
"value")` |
+| **Remove property** | `{"@type":"removeProperty","property":"key"}`
| `ModelChange.removeProperty("key")` |
`ModelChange.remove_property("key")` |
:::note
- Multiple modifications can be applied in a single request.
