cool9850311 commented on code in PR #6298:
URL: https://github.com/apache/gravitino/pull/6298#discussion_r1919869004


##########
core/src/main/java/org/apache/gravitino/tag/TagManager.java:
##########
@@ -352,6 +352,15 @@ public String[] associateTagsForMetadataObject(
                 }));
   }
 
+  private String getNewName(TagChange... changes) {
+    for (TagChange change : changes) {
+      if (change instanceof TagChange.RenameTag) {
+        return ((TagChange.RenameTag) change).getNewName();
+      }
+    }
+    return null;

Review Comment:
   The updated Tag entity is being constructed within a deeply nested callback 
function, making it challenging to access (since the error occurs, the updated 
Tag entity cannot be retrieved as a return value). Alternatively, I could 
include the error name in the exception thrown by the 
TagMetaService.updateTag() method.
   
   So I believe this alternative approach is simpler and more straightforward 
for retrieving the name.
   
   In this case, if a TagAlreadyExistsException is triggered, the method will 
never return null. However, instead of returning null in cases where the method 
might be misused, it might be better to throw an error to explicitly handle 
such situations.
   Which one do you prefer?
   Get the name from the Exception or TagChange?



-- 
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: commits-unsubscr...@gravitino.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to