[#5902] feat: Add tag failure event to Gravitino server #5944

cool9850311 · 2024-12-23T01:57:59Z

What changes were proposed in this pull request?

Add tag failure event to Gravitino server

Why are the changes needed?

Subtask: #5902

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit tests.

### What changes were proposed in this pull request? Add tag failure event to Gravitino server ### Why are the changes needed? Subtask: apache#5902 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unit tests.

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

FANNG1 · 2024-12-23T03:51:54Z

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

+
+public class TagFailureEvent extends FailureEvent {
+    public TagFailureEvent(String user, Exception exception) {
+        super(user, null, exception);


identifier should not be null for tag event.

What should I pass here?

FANNG1 · 2024-12-23T03:53:00Z

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

+  DELETE_TAG,
+  ALTER_TAG,
+  LIST_TAG,
+  TAG_EXISTS,


TAG_EXISTS not used? please remove it

FANNG1 · 2024-12-23T03:55:29Z

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

      throw e;
    }
  }

  @Override
  public Tag createTag(
      String metalake, String name, String comment, Map<String, String> properties) {
+      MetalakeInfo metalakeInfo = new MetalakeInfo(name, comment, properties, null);


we should create a TagInfo not MetalakeInfo here

@SuppressWarnings

- Removed @SuppressWarnings annotations. - Removed unused OperationType. - Replaced MetalakeInfo with TagInfo. - Refactored TagFailureEvent constructor.

FANNG1 · 2024-12-23T11:06:58Z

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

@@ -7,7 +7,7 @@ public class AlterTagFailureEvent extends TagFailureEvent {
    private final String name;
    private final TagChange[] changes;
    public AlterTagFailureEvent(String user, String metalake, String name, TagChange[] changes, Exception exception) {
-        super(user, exception);
+        super(user, null, exception);


please provide a meaning identifier to all failure event, for tag operations the resource identifier is the $metalake.$tagName, for list operations the the resource identifier is the objects which supports list tags, such as $metalake.$metadataObject in listTagsForMetadataObject

Do I need to replace all String metalake in the TagDispatcher interface, as well as the fields that have been replaced with NameIdentifier in the TagManager, with NameIdentifier and construct the NameIdentifier in operations instead?

what do you mean about Do I need to replace all String metalake in the TagDispatcher interface?

I mean, currently TagDispatcher's methods are passing metalake as a String, and then turn it to NameIdentifier in TagManager.
I'm asking about if I should turn it to NameIdentifier earlier in TagOperations and then pass it through event and manager.

I perfer not changing the interfaces in TagManager

FANNG1 · 2024-12-25T07:28:42Z

event describes what happened to a resource identified by a nameIdentifier,

for most tag event(create/get/delete), the identifier is the metalakeName.tagName which could be generated by ofTagIdent(String metalake, String tagName) ,
list tag event, the identifier the resource which supports list tags like metalake, or metadata object
for tag event with metadataObject, the identifier is the $metalake.$metadataObject

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

...java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectFailureEvent.java

xunliu · 2024-12-26T12:41:43Z

hi @FANNG1 Please help review this PR, thanks.

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

...java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectFailureEvent.java

FANNG1 · 2024-12-27T07:58:33Z

Could you fix the comments and could you add related document in gravitino-server-config.md?

cool9850311 · 2024-12-27T08:40:48Z

Could you fix the comments and could you add related document in gravitino-server-config.md?

OK

This commit cleans up the TagFailureEvent classes by removing the unused 'metalake', 'name' and 'metadataObject'fields, along with their associated methods. This refactoring simplifies the code and enhances maintainability.

cool9850311 · 2024-12-28T03:46:57Z

@FANNG1 Done, pls review thx

cool9850311 · 2024-12-28T16:13:53Z

@FANNG1 I run the Frontend IT test successfully in local, but it does not pass here 2 times, and it says connection timeout, do I need to change anything?

FANNG1 · 2024-12-29T00:56:06Z

@FANNG1 I run the Frontend IT test successfully in local, but it does not pass here 2 times, and it says connection timeout, do I need to change anything?

no, it's network failures, reruned it.

...main/java/org/apache/gravitino/listener/api/event/ListMetadataObjectsForTagFailureEvent.java

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

docs/gravitino-server-config.md

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

FANNG1 · 2025-01-07T02:22:55Z

@cool9850311 , LGTM except for minor comments, could you fix it?

FANNG1 · 2025-01-07T06:31:13Z

LGTM, @jerryshao do you have time to review again?

jerryshao

LGTM.

…#5944) ### What changes were proposed in this pull request? Add tag failure event to Gravitino server ### Why are the changes needed? Subtask: apache#5902 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unit tests.

FANNG1 reviewed Dec 23, 2024

View reviewed changes

Refactor codebase:

21aa71e

- Removed @SuppressWarnings annotations. - Removed unused OperationType. - Replaced MetalakeInfo with TagInfo. - Refactored TagFailureEvent constructor.

FANNG1 reviewed Dec 23, 2024

View reviewed changes

Add meaning identifier to all failure event

458a091

FANNG1 mentioned this pull request Dec 25, 2024

[#5900] feat(tag): support tag pre-event to Gravitino server #5980

Draft

FANNG1 reviewed Dec 25, 2024

View reviewed changes

core/src/main/java/org/apache/gravitino/listener/api/event/AlterTagFailureEvent.java Show resolved Hide resolved

FANNG1 reviewed Dec 25, 2024

View reviewed changes

...java/org/apache/gravitino/listener/api/event/AssociateTagsForMetadataObjectFailureEvent.java Show resolved Hide resolved

cool9850311 added 5 commits December 25, 2024 21:15

Change ident to metalake & tag name

9a33cf1

Add javadoc to all tag failure event

250a977

Format code

259295b

Add docs and license

f73787d

format code

4ccbe90

FANNG1 mentioned this pull request Dec 26, 2024

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

Draft

cool9850311 marked this pull request as ready for review December 27, 2024 02:25

FANNG1 reviewed Dec 27, 2024

View reviewed changes

cool9850311 added 3 commits December 27, 2024 20:22

refactor: move tag methods to util

572a823

refactor: remove unused fields from tag failure events

7341ee5

This commit cleans up the TagFailureEvent classes by removing the unused 'metalake', 'name' and 'metadataObject'fields, along with their associated methods. This refactoring simplifies the code and enhances maintainability.

update readme

9410eb5

fix test

42374d2

FANNG1 reviewed Jan 7, 2025

View reviewed changes

fix bugs

25ce87d

FANNG1 approved these changes Jan 7, 2025

View reviewed changes

jerryshao approved these changes Jan 7, 2025

View reviewed changes

jerryshao assigned cool9850311 Jan 7, 2025

jerryshao merged commit 9823d77 into apache:main Jan 7, 2025
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#5902] feat: Add tag failure event to Gravitino server #5944

[#5902] feat: Add tag failure event to Gravitino server #5944

cool9850311 commented Dec 23, 2024

FANNG1 Dec 23, 2024

cool9850311 Dec 23, 2024

FANNG1 Dec 23, 2024

FANNG1 Dec 23, 2024

FANNG1 Dec 23, 2024

cool9850311 Dec 23, 2024

cool9850311 Dec 24, 2024

FANNG1 Dec 24, 2024

cool9850311 Dec 24, 2024

FANNG1 Dec 24, 2024

cool9850311 Dec 24, 2024

FANNG1 commented Dec 25, 2024

xunliu commented Dec 26, 2024

FANNG1 commented Dec 27, 2024 •

edited

Loading

cool9850311 commented Dec 27, 2024

cool9850311 commented Dec 28, 2024

cool9850311 commented Dec 28, 2024 •

edited

Loading

FANNG1 commented Dec 29, 2024

FANNG1 commented Jan 7, 2025

FANNG1 commented Jan 7, 2025

jerryshao left a comment

[#5902] feat: Add tag failure event to Gravitino server #5944

[#5902] feat: Add tag failure event to Gravitino server #5944

Conversation

cool9850311 commented Dec 23, 2024

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

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 Dec 25, 2024

xunliu commented Dec 26, 2024

FANNG1 commented Dec 27, 2024 • edited Loading

cool9850311 commented Dec 27, 2024

cool9850311 commented Dec 28, 2024

cool9850311 commented Dec 28, 2024 • edited Loading

FANNG1 commented Dec 29, 2024

FANNG1 commented Jan 7, 2025

FANNG1 commented Jan 7, 2025

jerryshao left a comment

Choose a reason for hiding this comment

FANNG1 commented Dec 27, 2024 •

edited

Loading

cool9850311 commented Dec 28, 2024 •

edited

Loading