rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3355730317
##########
plan/src/main/java/org/apache/flink/agents/plan/actions/Action.java:
##########
@@ -33,36 +33,36 @@
import java.util.Objects;
/**
- * Representation of an agent action with event listening and function
execution.
+ * Representation of an agent action with unified trigger conditions.
*
- * <p>This class encapsulates a named agent action that listens for specific
event types and
- * executes an associated function when those events occur.
+ * <p>Each entry of {@code triggerConditions} is an event type name string.
Multiple entries combine
+ * with OR.
*/
@JsonSerialize(using = ActionJsonSerializer.class)
@JsonDeserialize(using = ActionJsonDeserializer.class)
public class Action {
private final String name;
private final Function exec;
- private final List<String> listenEventTypes;
+ private final List<String> triggerConditions;
Review Comment:
Thanks for the comment — this was actually my first instinct too: a clean
routing-vs-filtering separation. The more I worked through the problem though,
the more I came to think it doesn't quite map onto what users actually want to
express.
Event filtering is just event routing at a finer granularity, not a separate
concern. Each entry in `trigger_conditions` answers exactly one question —
"should this action fire for this event?" — and a bare event type is simply the
most common (and most degenerate) form of that condition.
The case that really pins this down: for a single action, users often want
**per-event-type conditions** — fire on type A unconditionally, fire on type B
only when condition B holds, fire on type C only when condition C holds. With a
unified list this falls out naturally, one entry per branch:
```
trigger_conditions: [
"event_type_a",
"type == 'event_type_b' && condition_b",
"type == 'event_type_c' && condition_c",
]
```
Split into `listen_event_types` + `trigger_conditions`, there's no clean way
to express the per-type correlation — the two fields are parallel lists with no
shared index, so "condition B belongs to type B, condition C to type C" has no
representation. You'd end up either forcing one CEL expression to gate all
listened types (loses per-type granularity), or inventing a dict like `{type_b:
condition_b, type_c: condition_c}` (which is just the unified list with extra
ceremony). The unified field is what naturally fits the actual shape of user
intent.
Curious if you have a concrete scenario in mind where the two-field split
would be cleaner — happy to revisit if there's a use case I'm missing.
--
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]
