[#5901] feat(core): support tag event to Gravitino server #5998

TungYuChiang · 2024-12-26T03:04:19Z

What changes were proposed in this pull request?

Create corresponding event like ListTagEvent
Add corresponding event to TagEventDispatcher

Why are the changes needed?

Fix: #5901

Does this PR introduce any user-facing change?

no

How was this patch tested?

orenccl · 2024-12-26T03:07:30Z

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

@@ -72,7 +85,10 @@ public Tag getTag(String metalake, String name) throws NoSuchTagException {
    // TODO: getTagPreEvent
    try {
      // TODO: getTagEvent


Redundant line?

orenccl · 2024-12-26T03:09:09Z

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

@@ -121,7 +150,11 @@ public MetadataObject[] listMetadataObjectsForTag(String metalake, String name)
    // TODO: listMetadataObjectsForTagPreEvent
    try {
      // TODO: listMetadataObjectsForTagEvent


Redundant line?

orenccl · 2024-12-26T06:37:52Z

It would be better to turn this PR into a draft before updating the unit tests.

FANNG1 · 2024-12-26T13:49:15Z

will review the PR until #5944 is merged for some duplicated logic, please wait for a while.

FANNG1 · 2025-01-07T11:57:01Z

@TungYuChiang could you rebase your code since #5944 is merged

FANNG1 · 2025-01-07T11:58:16Z

cc @cool9850311

TungYuChiang · 2025-01-08T03:25:53Z

@TungYuChiang could you rebase your code since #5944 is merged

sure, I will complete it within the next few days.

FANNG1 · 2025-01-13T01:44:34Z

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

-      // TODO: listTagsEvent
-      return dispatcher.listTags(metalake);
+      String[] tagNames = dispatcher.listTags(metalake);
+      eventBus.dispatchEvent(new ListTagEvent(PrincipalUtils.getCurrentUserName(), metalake));


could you rename ListTagsFailureEvent to ListTagFailureEvent to keep consistent with other list event?

could you rename ListTagsFailureEvent to ListTagFailureEvent to keep consistent with other list event?

@FANNG1 , Would it be better to rename ListTagEvent to ListTagsEvent?

Yes, ListTagsEvent is better

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

FANNG1 · 2025-01-13T01:57:36Z

Event represents an specific operation to specific resource, the current implement pass metalake as resource name, this's not correct for some event, please refer to XXTagFailureEvent for the definition of the resource.

FANNG1 · 2025-01-13T02:00:41Z

please update the document too

FANNG1 · 2025-01-15T02:19:33Z

@TungYuChiang any progress? If this PR is merged these days, we could meet the 0.8 release.

TungYuChiang · 2025-01-15T02:57:20Z

@TungYuChiang any progress? If this PR is merged these days, we could meet the 0.8 release.

@FANNG1 , I’m sorry for the delay. I’ve been a bit busy these past few days, but I’ll finish it by tomorrow evening.
Thank you for your patience

FANNG1 · 2025-01-15T11:14:26Z

@TungYuChiang any progress? If this PR is merged these days, we could meet the 0.8 release.

@FANNG1 , I’m sorry for the delay. I’ve been a bit busy these past few days, but I’ll finish it by tomorrow evening. Thank you for your patience

no hurry, please take your time

TungYuChiang · 2025-01-16T09:11:27Z

@FANNG1, I have fixed everything except renaming ListTagsInfoFailureEvent to ListTagInfoFailureEvent

- CreateTag - AlterTagEvent - GetTag - DeleteTag - ListTag - ListTagInfo

- [x] GetTagForMetadataObject - [x] ListMetadataObjectsForTag - [x] AssociateTagsForMetadataObject - [x] ListTagsForMetadataObject - [x] ListTagsInfoForMetadataObject

TungYuChiang · 2025-01-19T08:43:11Z

@FANNG1, I have fixed everything except renaming ListTagsInfoFailureEvent to ListTagInfoFailureEvent

FANNG1 · 2025-01-20T06:43:05Z

core/src/main/java/org/apache/gravitino/listener/api/event/GetTagEvent.java

+   * @param tag The {@link Tag} object representing the retrieved tag.
+   */
+  public GetTagEvent(String user, String metalake, String tagName, Tag tag) {
+    super(user, NameIdentifier.of(metalake));


Please use NameIdentifierUtil.ofTag(metalake, tagName) as resourceIdentifier.

FANNG1 · 2025-01-20T06:46:09Z

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

-      // TODO: getTagEvent
-      return dispatcher.getTag(metalake, name);
+      Tag tag = dispatcher.getTag(metalake, name);
+      eventBus.dispatchEvent(


please create TagInfo and passing TagInfo to GetTagEvent

FANNG1 · 2025-01-20T06:55:17Z

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

-      // TODO: listTagsEvent
-      return dispatcher.listTags(metalake);
+      String[] tagNames = dispatcher.listTags(metalake);
+      eventBus.dispatchEvent(new ListTagsEvent(PrincipalUtils.getCurrentUserName(), metalake));


please passing tagNames to ListTagsEvent

FANNG1 · 2025-01-20T06:59:03Z

core/src/main/java/org/apache/gravitino/listener/TagEventDispatcher.java

-      // TODO: getTagForMetadataObjectEvent
-      return dispatcher.getTagForMetadataObject(metalake, metadataObject, name);
+      Tag tag = dispatcher.getTagForMetadataObject(metalake, metadataObject, name);
+      eventBus.dispatchEvent(


please use TagInfo

FANNG1 · 2025-01-20T07:00:54Z

core/src/main/java/org/apache/gravitino/listener/api/event/AlterTagEvent.java

+  public AlterTagEvent(
+      String user, String metalake, TagChange[] tagChanges, TagInfo updatedTagInfo) {
+    super(user, NameIdentifier.of(metalake));
+    this.tagChanges = tagChanges;


Please use clone

this.tagChanges = tagChanges.clone();

FANNG1 · 2025-01-20T07:02:56Z

core/src/main/java/org/apache/gravitino/listener/api/event/AlterTagEvent.java

+   */
+  public AlterTagEvent(
+      String user, String metalake, TagChange[] tagChanges, TagInfo updatedTagInfo) {
+    super(user, NameIdentifier.of(metalake));


use NameIdentifierUtil.ofTag(metalake, tagName) as identifier

FANNG1 · 2025-01-20T07:03:58Z

...c/main/java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectEvent.java

+    super(user, NameIdentifier.of(metalake));
+    this.metalake = metalake;
+    this.metadataObject = metadataObject;
+    this.tagsToAdd = tagsToAdd;


use clone for tagsToAdd tagsToRemove associatedTags

FANNG1 · 2025-01-20T07:05:10Z

...c/main/java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectEvent.java

+      String[] tagsToAdd,
+      String[] tagsToRemove,
+      String[] associatedTags) {
+    super(user, NameIdentifier.of(metalake));


please use MetadataObjectUtil.toEntityIdent(metalake, metadataObject) as identifier

FANNG1 · 2025-01-20T07:05:58Z

...c/main/java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectEvent.java

+   *
+   * @return The {@link MetadataObject} with which the tags were associated.
+   */
+  public MetadataObject metadataObject() {


please remove this, duplicated with identifier

FANNG1 · 2025-01-20T07:06:16Z

...c/main/java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectEvent.java

+   *
+   * @return The metalake from which the tags were associated.
+   */
+  public String metalake() {


please remove this, duplicated with identifier

FANNG1 · 2025-01-20T07:08:02Z

Event represents an specific operation to specific resource, the current implement pass metalake as resource name, this's not correct for some event, please refer to XXTagFailureEvent for the definition of the resource.

@TungYuChiang , please refer to XXTagFailureEvent for the definition of the resource identifier, thx.

TungYuChiang · 2025-01-21T04:36:30Z

Event represents an specific operation to specific resource, the current implement pass metalake as resource name, this's not correct for some event, please refer to XXTagFailureEvent for the definition of the resource.

@TungYuChiang , please refer to XXTagFailureEvent for the definition of the resource identifier, thx.

I will fix it in the next few days.

orenccl assigned TungYuChiang Dec 26, 2024

orenccl requested a review from FANNG1 December 26, 2024 03:05

orenccl suggested changes Dec 26, 2024

View reviewed changes

xunliu marked this pull request as draft December 26, 2024 08:26

TungYuChiang force-pushed the feature/tagEvent branch from aba70be to 42e0f47 Compare December 26, 2024 13:04

TungYuChiang force-pushed the feature/tagEvent branch 4 times, most recently from a680fe6 to 43c2d20 Compare January 11, 2025 08:26

FANNG1 reviewed Jan 13, 2025

View reviewed changes

orenccl mentioned this pull request Jan 13, 2025

[#6078] feat(core): Support model event to Gravitino server #6129

Open

TungYuChiang force-pushed the feature/tagEvent branch from 43c2d20 to 22722e2 Compare January 16, 2025 00:49

TungYuChiang added 7 commits January 19, 2025 15:06

Add TagEvent

22bf87d

Add Create & List TagEvent

8fdc381

Add all tag event

fe52536

spotlessApply

07861b6

remove redundant line

84a7a8d

add unit test init for tagEvent & add createTagEvent test

5618df1

fix createTagTest and add listTagTest

e707a47

TungYuChiang added 6 commits January 19, 2025 15:06

add test

9ff3627

Add

1825dac

- CreateTag - AlterTagEvent - GetTag - DeleteTag - ListTag - ListTagInfo

Add

330e8e7

- [x] GetTagForMetadataObject - [x] ListMetadataObjectsForTag - [x] AssociateTagsForMetadataObject - [x] ListTagsForMetadataObject - [x] ListTagsInfoForMetadataObject

fix bugs

c8d62ec

Remove incorrect test

fa42784

Fix bugs of AlterTagEvent

2c66d22

TungYuChiang force-pushed the feature/tagEvent branch 2 times, most recently from 26214d8 to 2c66d22 Compare January 19, 2025 07:46

ListTagEvent to ListTagsEvent & ListTagInfoEvent to ListTagsInfoEvent

64a293c

TungYuChiang force-pushed the feature/tagEvent branch from 9740fcb to 64a293c Compare January 19, 2025 08:13

TungYuChiang closed this Jan 19, 2025

TungYuChiang reopened this Jan 19, 2025

FANNG1 reviewed Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#5901] feat(core): support tag event to Gravitino server #5998

[#5901] feat(core): support tag event to Gravitino server #5998

TungYuChiang commented Dec 26, 2024 •

edited

Loading

orenccl Dec 26, 2024

orenccl Dec 26, 2024

orenccl commented Dec 26, 2024

FANNG1 commented Dec 26, 2024

FANNG1 commented Jan 7, 2025

FANNG1 commented Jan 7, 2025

TungYuChiang commented Jan 8, 2025

FANNG1 Jan 13, 2025

TungYuChiang Jan 16, 2025 •

edited

Loading

FANNG1 Jan 16, 2025

FANNG1 commented Jan 13, 2025

FANNG1 commented Jan 13, 2025

FANNG1 commented Jan 15, 2025

TungYuChiang commented Jan 15, 2025 •

edited

Loading

FANNG1 commented Jan 15, 2025

TungYuChiang commented Jan 16, 2025

TungYuChiang commented Jan 19, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 Jan 20, 2025

FANNG1 commented Jan 20, 2025

TungYuChiang commented Jan 21, 2025

[#5901] feat(core): support tag event to Gravitino server #5998

Are you sure you want to change the base?

[#5901] feat(core): support tag event to Gravitino server #5998

Conversation

TungYuChiang commented Dec 26, 2024 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orenccl commented Dec 26, 2024

FANNG1 commented Dec 26, 2024

FANNG1 commented Jan 7, 2025

FANNG1 commented Jan 7, 2025

TungYuChiang commented Jan 8, 2025

Choose a reason for hiding this comment

TungYuChiang Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FANNG1 commented Jan 13, 2025

FANNG1 commented Jan 13, 2025

FANNG1 commented Jan 15, 2025

TungYuChiang commented Jan 15, 2025 • edited Loading

FANNG1 commented Jan 15, 2025

TungYuChiang commented Jan 16, 2025

TungYuChiang commented Jan 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FANNG1 commented Jan 20, 2025

TungYuChiang commented Jan 21, 2025

TungYuChiang commented Dec 26, 2024 •

edited

Loading

TungYuChiang Jan 16, 2025 •

edited

Loading

TungYuChiang commented Jan 15, 2025 •

edited

Loading