Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.gravitino.tag.TagChange;
import org.apache.gravitino.tag.TagDispatcher;
import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.gravitino.listener.api.info.TagInfo;

FANNG1 marked this conversation as resolved.
Show resolved Hide resolved
/**
* {@code TagEventDispatcher} is a decorator for {@link TagDispatcher} that not only delegates tag
Expand All @@ -46,10 +47,7 @@
* of tag operations.
*/
public class TagEventDispatcher implements TagDispatcher {
@SuppressWarnings("unused")
private final EventBus eventBus;

@SuppressWarnings("unused")
private final TagDispatcher dispatcher;

public TagEventDispatcher(EventBus eventBus, TagDispatcher dispatcher) {
Expand Down Expand Up @@ -96,13 +94,13 @@ public Tag getTag(String metalake, String name) throws NoSuchTagException {
@Override
public Tag createTag(
String metalake, String name, String comment, Map<String, String> properties) {
MetalakeInfo metalakeInfo = new MetalakeInfo(name, comment, properties, null);
// TODO: createTagPreEvent
TagInfo tagInfo = new TagInfo(name, comment, properties);
// TODO: createTagPreEvent
try {
// TODO: createTagEvent
return dispatcher.createTag(metalake, name, comment, properties);
} catch (Exception e) {
eventBus.dispatchEvent(new CreateTagFailureEvent(PrincipalUtils.getCurrentUserName(), metalake, metalakeInfo, e));
eventBus.dispatchEvent(new CreateTagFailureEvent(PrincipalUtils.getCurrentUserName(), metalake, tagInfo, e));
throw e;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I perfer not changing the interfaces in TagManager

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

this.name = name;
this.metalake = metalake;
this.changes = changes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public class AssociateTagsForMetadataObjectFailureEvent extends TagFailureEvent
private final String[] tagsToAdd;
private final String[] tagsToRemove;
public AssociateTagsForMetadataObjectFailureEvent(String user, String metalake, MetadataObject metadataObject, String[] tagsToAdd, String[] tagsToRemove, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
this.metadataObject = metadataObject;
this.tagsToAdd = tagsToAdd;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
package org.apache.gravitino.listener.api.event;

import org.apache.gravitino.listener.api.info.MetalakeInfo;
import org.apache.gravitino.listener.api.info.TagInfo;

public class CreateTagFailureEvent extends TagFailureEvent {
private final String metalake;
private final MetalakeInfo metalakeInfo;
public CreateTagFailureEvent(String user, String metalake, MetalakeInfo metalakeInfo, Exception exception) {
super(user, exception);
private final TagInfo tagInfo;
public CreateTagFailureEvent(String user, String metalake, TagInfo tagInfo, Exception exception) {
super(user, null, exception);
this.metalake = metalake;
this.metalakeInfo = metalakeInfo;
this.tagInfo = tagInfo;
}

public MetalakeInfo metalakeInfo() {
return metalakeInfo;
public TagInfo tagInfo() {
return tagInfo;
}

public String metalake() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public class DeleteTagFailureEvent extends TagFailureEvent {
private final String metalake;
private final String name;
public DeleteTagFailureEvent(String user, String metalake, String name, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
this.name = name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ public class GetTagFailureEvent extends TagFailureEvent {
private final String metalake;
private final String name;
public GetTagFailureEvent(String user, String metalake, String name, Exception exception) {
super(user, exception);
super(user, null, exception);
this.name = name;
this.metalake = metalake;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public class GetTagForMetadataObjectFailureEvent extends TagFailureEvent {
private final MetadataObject metadataObject;
private final String name;
public GetTagForMetadataObjectFailureEvent(String user, String metalake, MetadataObject metadataObject, String name, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
this.metadataObject = metadataObject;
this.name = name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ public class ListMetadataObjectsForTagFailureEvent extends TagFailureEvent {
private final String metalake;
private final String name;
public ListMetadataObjectsForTagFailureEvent(String user, String metalake, String name, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
this.name = name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
public class ListTagFailureEvent extends TagFailureEvent {
private final String metalake;
public ListTagFailureEvent(String user, String metalake, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ public class ListTagsForMetadataObjectFailureEvent extends TagFailureEvent {
private final String metalake;
private final MetadataObject metadataObject;
public ListTagsForMetadataObjectFailureEvent(String user, String metalake, MetadataObject metadataObject, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
this.metadataObject = metadataObject;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ public class ListTagsInfoForMetadataObjectFailureEvent extends TagFailureEvent {
private final String metalake;
private final MetadataObject metadataObject;
public ListTagsInfoForMetadataObjectFailureEvent(String user, String metalake, MetadataObject metadataObject, Exception exception) {
super(user, exception);
super(user, null, exception);
this.metalake = metalake;
this.metadataObject = metadataObject;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public enum OperationType {
DELETE_TAG,
ALTER_TAG,
LIST_TAG,
TAG_EXISTS,
ASSOCIATE_TAGS_FOR_METADATA_OBJECT,
LIST_TAGS_FOR_METADATA_OBJECT,
LIST_TAGS_INFO_FOR_METADATA_OBJECT,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package org.apache.gravitino.listener.api.event;
import org.apache.gravitino.NameIdentifier;

public class TagFailureEvent extends FailureEvent {
public TagFailureEvent(String user, Exception exception) {
super(user, null, exception);
public TagFailureEvent(String user, NameIdentifier identifier, Exception exception) {
super(user, identifier, exception);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package org.apache.gravitino.listener.api.info;

import com.google.common.collect.ImmutableMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.apache.gravitino.annotation.DeveloperApi;

/**
* Provides access to metadata about a Tag instance, designed for use by event listeners. This
* class encapsulates the essential attributes of a Tag, including its name, optional
* description, properties, and audit information.
*/
@DeveloperApi
public final class TagInfo {
private final String name;
@Nullable private final String comment;
private final Map<String, String> properties;


/**
* Directly constructs TagInfo with specified details.
*
* @param name The name of the Tag.
* @param comment An optional description for the Tag.
* @param properties A map of properties associated with the Tag.
*/
public TagInfo(String name, String comment, Map<String, String> properties) {
this.name = name;
this.comment = comment;
this.properties = properties == null ? ImmutableMap.of() : ImmutableMap.copyOf(properties);
}


/**
* Returns the name of the Tag.
*
* @return The Tag's name.
*/
public String name() {
return name;
}

/**
* Returns the optional comment describing the Tag.
*
* @return The comment, or null if not provided.
*/
@Nullable
public String comment() {
return comment;
}

/**
* Returns the properties of the Tag.
*
* @return A map of Tag properties.
*/
public Map<String, String> properties() {
return properties;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ void testCreateTagFailureEvent() {
GravitinoRuntimeException.class, ((CreateTagFailureEvent) event).exception().getClass()
);
Assertions.assertEquals("metalake", ((CreateTagFailureEvent) event).metalake());
Assertions.assertEquals(tag.name(), ((CreateTagFailureEvent) event).metalakeInfo().name());
Assertions.assertEquals(tag.comment(), ((CreateTagFailureEvent) event).metalakeInfo().comment());
Assertions.assertEquals(tag.properties(), ((CreateTagFailureEvent) event).metalakeInfo().properties());
Assertions.assertEquals(tag.name(), ((CreateTagFailureEvent) event).tagInfo().name());
Assertions.assertEquals(tag.comment(), ((CreateTagFailureEvent) event).tagInfo().comment());
Assertions.assertEquals(tag.properties(), ((CreateTagFailureEvent) event).tagInfo().properties());
Assertions.assertEquals(OperationType.CREATE_TAG, event.operationType());
Assertions.assertEquals(OperationStatus.FAILURE, event.operationStatus());
}
Expand Down
Loading