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