wenjin272 commented on code in PR #631:
URL: https://github.com/apache/flink-agents/pull/631#discussion_r3121308458
##########
api/src/main/java/org/apache/flink/agents/api/event/ChatRequestEvent.java:
##########
@@ -27,31 +28,37 @@
/** Event representing a request for chat. */
public class ChatRequestEvent extends Event {
- private final String model;
- private final List<ChatMessage> messages;
- private final @Nullable Object outputSchema;
+
+ public static final String EVENT_TYPE = "_chat_request_event";
public ChatRequestEvent(
String model, List<ChatMessage> messages, @Nullable Object
outputSchema) {
- this.model = model;
- this.messages = messages;
- this.outputSchema = outputSchema;
+ super(EVENT_TYPE);
+ setAttr("model", model);
+ setAttr("messages", messages);
+ if (outputSchema != null) {
+ setAttr("output_schema", outputSchema);
+ }
}
public ChatRequestEvent(String model, List<ChatMessage> messages) {
this(model, messages, null);
}
+ @JsonIgnore
public String getModel() {
- return model;
+ return (String) getAttr("model");
}
+ @JsonIgnore
+ @SuppressWarnings("unchecked")
public List<ChatMessage> getMessages() {
- return messages;
+ return (List<ChatMessage>) getAttr("messages");
Review Comment:
In the python `ChatRequestEvent`, we deserialize the `messages` field in
`from_event` method. But the java `Event` is missing corresponding method. This
may cause `ClassCastException`.
##########
api/src/main/java/org/apache/flink/agents/api/EventContext.java:
##########
@@ -25,27 +25,44 @@
/** Contextual information about an event, such as its type and timestamp. */
public class EventContext {
- // Type of the event, the class name of the event
+ /** The routing key for the event, matching the {@code EVENT_TYPE}
constant or type string. */
private final String eventType;
+
+ /**
+ * The fully qualified Java class name used for deserialization. For
unified events (no
+ * subclass), this is {@code "org.apache.flink.agents.api.Event"}.
+ */
+ private final String eventClass;
Review Comment:
I think `EventContext` does not need to know the specific event class.
This information will enable us to restore the specific subclass type when
reconstructing events from the event log. However, I believe there is no
requirement to deserialize events from the event log. Furthermore, being aware
of specific event classes will make the framework's implementation more
complex.
I think that if users utilize event subclasses instead of the base event
directly, they should be responsible for handling event deserialization,
specifically by uniformly using `from_event`/`fromEvent`.
##########
api/src/main/java/org/apache/flink/agents/api/Event.java:
##########
@@ -20,35 +20,59 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;
/** Base class for all event types in the system. */
-public abstract class Event {
+public class Event {
Review Comment:
The python `Event` contains a class method `from_event`. In this method, we
will perform field validation and deserialization. However, Java's event lacks
the corresponding method.
##########
api/src/main/java/org/apache/flink/agents/api/Event.java:
##########
@@ -73,17 +97,34 @@ public void setSourceTimestamp(long timestamp) {
this.sourceTimestamp = timestamp;
}
+ /**
+ * Creates an Event from a JSON string.
+ *
+ * @param json the JSON string to deserialize
+ * @return the deserialized Event
+ * @throws IOException if JSON parsing fails
+ * @throws IllegalArgumentException if the 'type' field is missing or empty
+ */
+ public static Event fromJson(String json) throws IOException {
+ Event event = MAPPER.readValue(json, Event.class);
+ if (event.type == null || event.type.isEmpty()) {
+ throw new IllegalArgumentException("Event JSON must contain a
'type' field.");
Review Comment:
I think we can move this validation to the construct method annotated
`@JsonCreator`. This ensures that all code paths constructing an Event will
enforce this validation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
