xintongsong commented on code in PR #45:
URL: https://github.com/apache/flink-agents/pull/45#discussion_r2184143705
##########
plan/src/main/java/org/apache/flink/agents/plan/WorkflowPlan.java:
##########
@@ -18,24 +18,32 @@
package org.apache.flink.agents.plan;
-import org.apache.flink.agents.api.Event;
+import org.apache.flink.agents.plan.serializer.WorkflowPlanJsonDeserializer;
+import org.apache.flink.agents.plan.serializer.WorkflowPlanJsonSerializer;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.List;
import java.util.Map;
/** Workflow plan compiled from user defined workflow. */
+@JsonSerialize(using = WorkflowPlanJsonSerializer.class)
+@JsonDeserialize(using = WorkflowPlanJsonDeserializer.class)
public class WorkflowPlan {
- private final Map<Class<? extends Event>, List<Action>> actions;
+ private final Map<String, Action> actions;
+ private final Map<String, List<Action>> eventTriggerActions;
Review Comment:
```suggestion
private final Map<String, Action> actions; // actionName -> action
private final Map<String, List<Action>> actionsByEvent; //
eventClassName -> actions
```
##########
plan/src/main/java/org/apache/flink/agents/plan/WorkflowPlan.java:
##########
@@ -18,24 +18,32 @@
package org.apache.flink.agents.plan;
-import org.apache.flink.agents.api.Event;
+import org.apache.flink.agents.plan.serializer.WorkflowPlanJsonDeserializer;
+import org.apache.flink.agents.plan.serializer.WorkflowPlanJsonSerializer;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.List;
import java.util.Map;
/** Workflow plan compiled from user defined workflow. */
+@JsonSerialize(using = WorkflowPlanJsonSerializer.class)
+@JsonDeserialize(using = WorkflowPlanJsonDeserializer.class)
public class WorkflowPlan {
- private final Map<Class<? extends Event>, List<Action>> actions;
+ private final Map<String, Action> actions;
+ private final Map<String, List<Action>> eventTriggerActions;
- public WorkflowPlan(Map<Class<? extends Event>, List<Action>> actions) {
+ public WorkflowPlan(
+ Map<String, Action> actions, Map<String, List<Action>>
eventTriggerActions) {
this.actions = actions;
+ this.eventTriggerActions = eventTriggerActions;
}
- public List<Action> getAction(Class<? extends Event> type) {
- return actions.get(type);
+ public Map<String, Action> getActions() {
+ return actions;
}
- public Map<Class<? extends Event>, List<Action>> getActions() {
- return actions;
+ public Map<String, List<Action>> getEventTriggerActions() {
Review Comment:
```suggestion
public Map<String, List<Action>> getActionsByEvent() {
```
##########
plan/src/main/java/org/apache/flink/agents/plan/PythonFunction.java:
##########
@@ -48,7 +48,13 @@ public Object call(Object... args) throws Exception {
// TODO: check Python function signature compatibility with given
parameter types
@Override
- public void checkSignature(Class<?>[] parameterTypes) throws Exception {
- throw new UnsupportedOperationException();
+ public void checkSignature(Class<?>[] parameterTypes) throws Exception {}
Review Comment:
What is the plan for implementation of this method? Shall we at least create
a issue for this? Because the todo comment can easily get overlooked.
##########
plan/src/main/java/org/apache/flink/agents/plan/WorkflowPlan.java:
##########
@@ -18,24 +18,32 @@
package org.apache.flink.agents.plan;
-import org.apache.flink.agents.api.Event;
+import org.apache.flink.agents.plan.serializer.WorkflowPlanJsonDeserializer;
+import org.apache.flink.agents.plan.serializer.WorkflowPlanJsonSerializer;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.List;
import java.util.Map;
/** Workflow plan compiled from user defined workflow. */
+@JsonSerialize(using = WorkflowPlanJsonSerializer.class)
+@JsonDeserialize(using = WorkflowPlanJsonDeserializer.class)
public class WorkflowPlan {
- private final Map<Class<? extends Event>, List<Action>> actions;
+ private final Map<String, Action> actions;
+ private final Map<String, List<Action>> eventTriggerActions;
Review Comment:
I'm aware that the current field name `eventTriggerActions` is aligned with
the python workflow plan. But I think `actionsByEvent` is better for
readability. Maybe we can have a separate follow-up commit to fix this for both
java and python.
--
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]
