You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "rosemarYuan (via GitHub)" <gi...@apache.org> on 2026/06/04 07:20:33 UTC
[PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
rosemarYuan opened a new pull request, #756:
URL: https://github.com/apache/flink-agents/pull/756
Linked issue: #754
### Purpose of change
This PR introduces `EventType` constants and a registry for built-in and user-defined event types, and unifies the Action trigger entry point across Java and Python:
- Java: `@Action(value = ...)`
- Python: `@action(*trigger_conditions)`
This is a preparatory API and plan change for the CEL Action Condition filtering feature tracked in #754. Follow-up PRs will add Java runtime CEL evaluation, Python runtime CEL evaluation, and documentation.
### Scope
Since #709, `@Action` / `@action` carry two orthogonal concerns:
| concern | Java element | Python API | decides |
| --- | --- | --- | --- |
| trigger | `value()` | `*trigger_conditions` | when the action fires |
| dispatch | `target()` | `target=` | where it runs |
This PR only changes the trigger side:
- Java: `listenEventTypes` is renamed to `value()`.
- Python: `*listen_events` is renamed to `*trigger_conditions`.
The dispatch side is unchanged. `target`, `PythonFunction`, and cross-language dispatch semantics are reused as-is.
### Tests
- Java: `EventTypeTest`, `AgentPlan*Test`, `Action*SerializerTest` — all green
- Python: `api/tests` + `plan/tests` — 218 passed, 11 skipped
- Verified that cross-language `target` dispatch still works after the trigger rename
### API
This is a source-level API rename on the trigger side.
Java:
- `@Action(listenEventTypes = {X.EVENT_TYPE})` → `@Action(EventType.X)`
- `@Action(listenEventTypes =..., target = @PythonFunction(...))` → `@Action(value = ..., target = @PythonFunction(...))`
Python:
- `@action(*listen_events, target=None)` → `@action(*trigger_conditions, target=None)`
- `target=` remains unchanged.
All in-tree Java and Python callers have been migrated.
- [x] `doc-included`
Updated 10 markdown files under `docs/content/docs/` to use the new `EventType.X` / `@Action(EventType.X)` form while preserving the existing cross-language `target` examples.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3497250533
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -184,27 +184,27 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
private void extractActions(
String actionName,
- String[] listenEventTypeStrings,
+ String[] triggerEntries,
org.apache.flink.agents.plan.Function function,
Map<String, Object> config)
throws Exception {
- List<String> eventTypeNames = new ArrayList<>(Arrays.asList(listenEventTypeStrings));
+ List<String> triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries));
- if (eventTypeNames.isEmpty()) {
+ if (triggerConditions.isEmpty()) {
throw new IllegalArgumentException(
"Action "
+ actionName
- + " must specify at least one event type via listenEventTypes.");
+ + " must specify at least one trigger entry via @Action(value = ...).");
Review Comment:
**Thanks.** Addressed all three in the latest revision: dropped the null guard (trigger_conditions is always present), renamed the field in the remaining three fixtures, and switched the error message to @Action(EventType.x). PTAL.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3360986369
##########
plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java:
##########
@@ -66,10 +66,15 @@ public Action deserialize(JsonParser jsonParser, DeserializationContext deserial
throw new IOException("Unsupported function type: " + funcType);
}
- // Deserialize listenEventTypes
- List<String> listenEventTypes = new ArrayList<>();
- node.get("listen_event_types")
- .forEach(eventTypeNode -> listenEventTypes.add(eventTypeNode.asText()));
+ // Deserialize trigger_conditions (fall back to legacy listen_event_types for older JSONs)
+ List<String> triggerConditions = new ArrayList<>();
+ JsonNode triggerNode = node.get("trigger_conditions");
+ if (triggerNode == null) {
+ triggerNode = node.get("listen_event_types");
Review Comment:
This fallback to the legacy `listen_event_types` key is the load-bearing guarantee that plan JSON persisted in older Flink state still deserializes after the rename — but nothing exercises it. The cross-version snapshot resources were all regenerated to `trigger_conditions` in this PR, so the compat tests now compare new-format against new-format and this branch never fires under test. A future refactor could delete the fallback and every test would stay green.
Worth a small deserialize test that feeds a JSON blob carrying `listen_event_types` and asserts it lands in `getTriggerConditions()` / `getListenEventTypes()`? That pins the old-key path so it can't be silently dropped.
##########
api/src/test/java/org/apache/flink/agents/api/EventTypeTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+/** Tests for {@link EventType}. */
+class EventTypeTest {
+
+ @Test
+ void builtInConstantsAreNonNull() {
Review Comment:
`assertNotNull` on inlined `String` constants can't really fail — the constants are non-null by construction — so this test passes regardless of what each constant holds. A copy-paste slip like `OutputEvent = InputEvent.EVENT_TYPE` would sail through.
Since the point of `EventType` is to be a single source of truth, would asserting value-equality against each source (e.g. `assertEquals(InputEvent.EVENT_TYPE, EventType.InputEvent)`) be a stronger check? That actually guards the mapping. Same applies to the Python `test_event_type.py` smoke test.
##########
python/flink_agents/api/tests/test_event_type.py:
##########
@@ -0,0 +1,38 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#################################################################################
+"""Smoke tests for :mod:`flink_agents.api.events.event_type`."""
+
+from __future__ import annotations
+
+from flink_agents.api.events.event_type import EventType
+
+
+def test_builtin_constants_are_non_empty_strings() -> None:
Review Comment:
This asserts each constant is a non-empty string, which holds for any string value and so won't catch a wrong mapping (e.g. two constants pointing at the same source `EVENT_TYPE`). Would asserting each constant against its source event's `EVENT_TYPE` be a better guard on the single-source-of-truth contract? Mirrors the suggestion on the Java `EventTypeTest`, so whatever shape you land on there can carry over here.
##########
python/flink_agents/plan/actions/action.py:
##########
@@ -71,7 +81,13 @@ def __serialize_config(self, config: Dict[str, Any]) -> Dict[str, Any] | None:
@model_validator(mode="before")
def __custom_deserialize(self) -> "Action":
- config = self["config"]
+ # Legacy fallback: listen_event_types → trigger_conditions
+ if "trigger_conditions" not in self or self.get("trigger_conditions") is None:
+ if self.get("listen_event_types"):
+ self["trigger_conditions"] = list(self["listen_event_types"])
+ self.pop("listen_event_types", None)
Review Comment:
Same thought as the Java side: this `model_validator` is the only thing keeping older persisted plans (with `listen_event_types`) deserializable, but no test feeds it the old key — the regenerated snapshot fixtures all use `trigger_conditions` now, so this branch is untested.
Would a small round-trip test help here — construct an `Action` from a dict with `listen_event_types` and assert it surfaces as `trigger_conditions`? That guards the fallback against a future cleanup.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "xintongsong (via GitHub)" <gi...@apache.org>.
xintongsong commented on PR #756:
URL: https://github.com/apache/flink-agents/pull/756#issuecomment-4739458107
I think there's no need to stay compatible for agent plans from older version. First of all, we are still in 0.x versions and should avoid carry any burdens due to trying to stay compatible with immature legacy apis. And secondly, the json agent plan should not be considered as an public API. It's internal. We should not support executing a json agent plan generated with an older version.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on PR #756:
URL: https://github.com/apache/flink-agents/pull/756#issuecomment-4678184138
Hi @xintongsong, need a policy call from you.
This PR renames `listen_event_types` -> `trigger_conditions`. I added a fallback on both Java (`ActionJsonDeserializer`) and Python (`Action.model_validator`), so old plan JSON still deserializes.
@wenjin272 pointed out we're still in 0.x and don't owe API/state compatibility yet (#631 already broke API in 0.3), so the fallback may be debt we shouldn't take on until the 0.4 / 1.0 commitment.
That policy angle wasn't on the table in the earlier review — we only discussed whether the fallback was correctly implemented. Could you decide:
(A) keep the fallback + tests, or
(B) drop them under the 0.x "no compat" stance?
Thanks!
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3361315282
##########
python/flink_agents/plan/actions/action.py:
##########
@@ -71,7 +81,13 @@ def __serialize_config(self, config: Dict[str, Any]) -> Dict[str, Any] | None:
@model_validator(mode="before")
def __custom_deserialize(self) -> "Action":
- config = self["config"]
+ # Legacy fallback: listen_event_types → trigger_conditions
+ if "trigger_conditions" not in self or self.get("trigger_conditions") is None:
+ if self.get("listen_event_types"):
+ self["trigger_conditions"] = list(self["listen_event_types"])
+ self.pop("listen_event_types", None)
Review Comment:
Same idea on the Python side. While adding the test I found that the custom __init__ was rejecting the old key before model_validator(mode="before") had a chance to rename it — so the fallback was silently broken. Fixed __init__ to accept **kwargs and handle the legacy key, and added a matching deserialization test.
##########
api/src/test/java/org/apache/flink/agents/api/EventTypeTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+/** Tests for {@link EventType}. */
+class EventTypeTest {
+
+ @Test
+ void builtInConstantsAreNonNull() {
Review Comment:
You're right — assertNotNull on inlined constants is vacuous. Originally addressed in a follow-up PR; now pulled forward into this commit with assertEquals(InputEvent.EVENT_TYPE, EventType.InputEvent) and similar for all 8 built-ins.
##########
python/flink_agents/api/tests/test_event_type.py:
##########
@@ -0,0 +1,38 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#################################################################################
+"""Smoke tests for :mod:`flink_agents.api.events.event_type`."""
+
+from __future__ import annotations
+
+from flink_agents.api.events.event_type import EventType
+
+
+def test_builtin_constants_are_non_empty_strings() -> None:
Review Comment:
Mirrors the Java fix — the Python test now asserts EventType.InputEvent == InputEvent.EVENT_TYPE directly for all 8 constants. Consistent shape across both languages.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3367937968
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -184,27 +184,27 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
private void extractActions(
String actionName,
- String[] listenEventTypeStrings,
+ String[] triggerEntries,
org.apache.flink.agents.plan.Function function,
Map<String, Object> config)
throws Exception {
- List<String> eventTypeNames = new ArrayList<>(Arrays.asList(listenEventTypeStrings));
+ List<String> triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries));
- if (eventTypeNames.isEmpty()) {
+ if (triggerConditions.isEmpty()) {
throw new IllegalArgumentException(
"Action "
+ actionName
- + " must specify at least one event type via listenEventTypes.");
+ + " must specify at least one trigger entry via @Action(value = ...).");
}
// Create an Action
- Action action = new Action(actionName, function, eventTypeNames, config);
+ Action action = new Action(actionName, function, triggerConditions, config);
// Add to actions map
actions.put(action.getName(), action);
// Add to actionsByEvent map
- for (String eventTypeName : eventTypeNames) {
+ for (String eventTypeName : triggerConditions) {
Review Comment:
Agreed, fine to leave as-is — the three sites are equivalent today and nothing can carry a CEL string yet. Resolved on my end.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "wzhero1 (via GitHub)" <gi...@apache.org>.
wzhero1 commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3355556754
##########
python/flink_agents/plan/actions/action.py:
##########
@@ -29,29 +29,39 @@
class Action(BaseModel):
- """Representation of an agent action with event listening and function execution.
+ """Representation of an agent action with unified trigger conditions.
- This class encapsulates a named agent action that listens for specific event
- types and executes an associated function when those events occur.
+ This class encapsulates a named agent action that triggers on matching
+ events and executes an associated function.
Attributes:
----------
name : str
Name/identifier of the agent Action.
exec : Function
To be executed when the Action is triggered.
- listen_event_types : List[str]
- List of event types that will trigger this Action's execution.
+ trigger_conditions : List[str]
+ Event-type name strings that will trigger this Action. Multiple
+ entries combine with OR semantics.
"""
model_config = ConfigDict(arbitrary_types_allowed=True)
name: str
# TODO: Raise a warning when the action has a return value, as it will be ignored.
exec: PythonFunction | JavaFunction
- listen_event_types: List[str]
+ trigger_conditions: List[str]
Review Comment:
**Python Action model lacks backward compat for old JSON key**
Java `ActionJsonDeserializer` (L71-73) falls back to `listen_event_types` when `trigger_conditions` is absent. The Python pydantic `Action` model only declares `trigger_conditions: List[str]` with no alias or model_validator fallback — deserializing old-format plan JSON from persisted Flink state will raise `ValidationError`.
##########
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:
Discuss：**Keep `listen_event_types`, add `trigger_conditions` as a new field**
Rather than renaming `listen_event_types` → `trigger_conditions`, I'd suggest keeping both:
- `listen_event_types`: which events route to this action (static dispatch)
- `trigger_conditions`: CEL expression filter for whether to process a matched event (dynamic filtering)
These are orthogonal concerns — routing vs. filtering. Keeping `listen_event_types` also avoids JSON backward-compat issues entirely (the Python pydantic model currently lacks the fallback that the Java deserializer has for the old key).
##########
api/src/main/java/org/apache/flink/agents/api/EventType.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+/**
+ * Compile-time constants for built-in event types and a runtime registry for user-defined events.
+ *
+ * <p>Usage in {@code @Action}:
+ *
+ * <ul>
+ * <li>Built-in events: {@code @Action(EventType.InputEvent)}
+ * <li>User-defined events: {@code @Action("MyCustomEvent")}
+ * </ul>
+ *
+ * <p>Resolution via {@link #lookupOrSelf}: built-in → user-registered → passthrough.
+ */
+public final class EventType {
Review Comment:
**Reconsider the `EventType` aggregation class**
`EventType.InputEvent` duplicates `InputEvent.EVENT_TYPE` — two sources of truth for the same value. The `register()` / `lookup()` / `lookupOrSelf()` registry has zero callers in production code today.
`InputEvent.EVENT_TYPE` is already self-documenting and lives in its natural namespace. Suggest deferring the registry until the CEL PR actually needs it, and just using the event class constants directly.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
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, with different conditions for different types — and sometimes multiple conditions on the same type. With a unified list this falls out naturally. [#726](https://github.com/apache/flink-agents/issues/726) shows the canonical shape:
```
// Mixed triggers. Entries are OR'ed.
@Action({
EventType.InputEvent, // direct type
"type == EventType.ChatResponseEvent && retryCount > 0", // metadata condition
"type == EventType.ToolResponseEvent && response.success == false", // payload condition
"type == EventType.ChatResponseEvent && response.plantype.contains('Day')" // metadata + payload
})
```
Split into` listen_event_types + trigger_conditions`, there's no clean way to express any of this. The two fields are parallel lists with no shared index, so "this condition belongs to that type" has no representation — and "ChatResponseEvent has two distinct conditions" is even less expressible. You'd end up either forcing one CEL expression to gate all listened types (loses per-type granularity), or inventing a multi-map like `{ChatResponseEvent: [cond1, cond2], ToolResponseEvent: [cond3]}` — which is just the unified list with extra ceremony, plus an awkward way to represent the "no condition" case for direct-type entries. 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3367937968
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -184,27 +184,27 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
private void extractActions(
String actionName,
- String[] listenEventTypeStrings,
+ String[] triggerEntries,
org.apache.flink.agents.plan.Function function,
Map<String, Object> config)
throws Exception {
- List<String> eventTypeNames = new ArrayList<>(Arrays.asList(listenEventTypeStrings));
+ List<String> triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries));
- if (eventTypeNames.isEmpty()) {
+ if (triggerConditions.isEmpty()) {
throw new IllegalArgumentException(
"Action "
+ actionName
- + " must specify at least one event type via listenEventTypes.");
+ + " must specify at least one trigger entry via @Action(value = ...).");
}
// Create an Action
- Action action = new Action(actionName, function, eventTypeNames, config);
+ Action action = new Action(actionName, function, triggerConditions, config);
// Add to actions map
actions.put(action.getName(), action);
// Add to actionsByEvent map
- for (String eventTypeName : eventTypeNames) {
+ for (String eventTypeName : triggerConditions) {
Review Comment:
Agreed, fine to leave as-is — the three sites are equivalent today and nothing can carry a CEL string yet. Thanks for the context, resolved on my end.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on PR #756:
URL: https://github.com/apache/flink-agents/pull/756#issuecomment-4637523361
Thanks for the quick turnaround — went through all four threads against the head commit and they're fully resolved.
Both legacy-key fallbacks now have a focused test that feeds the old `listen_event_types` key and asserts it surfaces as `getTriggerConditions()` / `trigger_conditions`, so the branch can't be silently dropped in a later cleanup. The `EventType` tests now assert value-equality against each source `EVENT_TYPE` on both sides, which actually guards the single-source-of-truth mapping. The `__init__` fix is the one worth calling out — catching that the custom `__init__` dropped the legacy key before `model_validator(mode="before")` ran turned a coverage gap into a real correctness fix. Nice find.
CI is green across the board. One forward-looking question before I'm fully comfortable — not a blocker for this PR:
The `getListenEventTypes()` / `listen_event_types` accessor is documented as the future filtering seam — its javadoc in `plan/.../actions/Action.java` says a later change overrides it to "filter out non-type entries" once CEL expressions land. But the `actionsByEvent` / `actions_by_event` routing maps are built through that accessor at only one of three sites:
- `AgentPlan.java:207` (user `@Action` methods) iterates the raw `triggerConditions` list directly.
- `AgentPlan.java:217` (`addBuiltAction`, built-ins) goes through `action.getListenEventTypes()`.
- `agent_plan.py:148` (Python) iterates the raw `action.trigger_conditions` directly.
Today the three are equivalent, since every trigger entry is a plain event-type name. The forward-looking concern: once the accessor starts filtering, the two raw-list sites — which are exactly the ones handling user-supplied conditions — would register CEL expression strings like `"type == EventType.ChatResponseEvent && retryCount > 0"` as literal keys in the routing map, which is what the accessor exists to strip. The built-in path that does go through the accessor never carries CEL, so the seam as drawn protects the one path that doesn't need it and bypasses the two that do.
I'm confident the three sites diverge today; less sure whether it bites, since that depends on the unmerged CEL PR's design. Would it make sense to route all three through the accessor now — or, equally, defer the accessor and read the raw field everywhere until the CEL PR needs it? Either way, the three sites agreeing seems like the property worth locking in before CEL builds on top.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3367031682
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -184,27 +184,27 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
private void extractActions(
String actionName,
- String[] listenEventTypeStrings,
+ String[] triggerEntries,
org.apache.flink.agents.plan.Function function,
Map<String, Object> config)
throws Exception {
- List<String> eventTypeNames = new ArrayList<>(Arrays.asList(listenEventTypeStrings));
+ List<String> triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries));
- if (eventTypeNames.isEmpty()) {
+ if (triggerConditions.isEmpty()) {
throw new IllegalArgumentException(
"Action "
+ actionName
- + " must specify at least one event type via listenEventTypes.");
+ + " must specify at least one trigger entry via @Action(value = ...).");
}
// Create an Action
- Action action = new Action(actionName, function, eventTypeNames, config);
+ Action action = new Action(actionName, function, triggerConditions, config);
// Add to actions map
actions.put(action.getName(), action);
// Add to actionsByEvent map
- for (String eventTypeName : eventTypeNames) {
+ for (String eventTypeName : triggerConditions) {
Review Comment:
Thanks for flagging this. The inconsistency wasn't intentional — when writing `extractActions`, the `triggerConditions` local variable was right at hand so I used it directly; same on the Python. Since all three return identical values in this PR, the divergence slipped through unnoticed. A simple oversight on my part.
The follow-up CEL PR consolidates all three sites into a single `registerAction() `method that routes exclusively via `getListenEventTypes()`, which by then performs real filtering. So the three sites end up in lockstep once CEL lands.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3359886351
##########
python/flink_agents/plan/actions/action.py:
##########
@@ -29,29 +29,39 @@
class Action(BaseModel):
- """Representation of an agent action with event listening and function execution.
+ """Representation of an agent action with unified trigger conditions.
- This class encapsulates a named agent action that listens for specific event
- types and executes an associated function when those events occur.
+ This class encapsulates a named agent action that triggers on matching
+ events and executes an associated function.
Attributes:
----------
name : str
Name/identifier of the agent Action.
exec : Function
To be executed when the Action is triggered.
- listen_event_types : List[str]
- List of event types that will trigger this Action's execution.
+ trigger_conditions : List[str]
+ Event-type name strings that will trigger this Action. Multiple
+ entries combine with OR semantics.
"""
model_config = ConfigDict(arbitrary_types_allowed=True)
name: str
# TODO: Raise a warning when the action has a return value, as it will be ignored.
exec: PythonFunction | JavaFunction
- listen_event_types: List[str]
+ trigger_conditions: List[str]
Review Comment:
Fixed in lines 84-88. Added a model_validator fallback that mirrors the Java ActionJsonDeserializer behavior:
```
# Legacy fallback: listen_event_types → trigger_conditions
if "trigger_conditions" not in self or self.get("trigger_conditions") is None:
if self.get("listen_event_types"):
self["trigger_conditions"] = list(self["listen_event_types"])
self.pop("listen_event_types", None)
```
Old-format plan JSON with listen_event_types will now deserialize without ValidationError.
##########
api/src/main/java/org/apache/flink/agents/api/EventType.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+/**
+ * Compile-time constants for built-in event types and a runtime registry for user-defined events.
+ *
+ * <p>Usage in {@code @Action}:
+ *
+ * <ul>
+ * <li>Built-in events: {@code @Action(EventType.InputEvent)}
+ * <li>User-defined events: {@code @Action("MyCustomEvent")}
+ * </ul>
+ *
+ * <p>Resolution via {@link #lookupOrSelf}: built-in → user-registered → passthrough.
+ */
+public final class EventType {
Review Comment:
Agreed. I've refactored EventType to be a pure namespace of constants — each one simply re-exports the value from the corresponding event class (e.g., EventType.InputEvent = _InputEvent.EVENT_TYPE), so there's a single source of truth. The register() / lookup() / lookupOrSelf() registry has been removed from this PR and will be introduced later when the CEL PR actually requires dynamic resolution.
Meanwhile, I've also slimmed down the EventType class by removing the internal utility functions — their concrete implementations are deferred to a follow-up PR.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3394016303
##########
api/src/main/java/org/apache/flink/agents/api/annotation/Action.java:
##########
@@ -59,12 +60,8 @@
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface Action {
- /**
- * List of event type strings that this action should respond to.
- *
- * @return Array of event type strings
- */
- String[] listenEventTypes();
+ /** Event type name strings; multiple entries have OR semantics. */
+ String[] value();
Review Comment:
Good point — added a one-sentence note on explaining:
- (1) The JLS §9.7.3 reason for the name.
- (2) Its mapping to Python's `*trigger_conditions`.
Kept it tight to avoid bloating the annotation javadoc.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3366740037
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -184,27 +184,27 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
private void extractActions(
String actionName,
- String[] listenEventTypeStrings,
+ String[] triggerEntries,
org.apache.flink.agents.plan.Function function,
Map<String, Object> config)
throws Exception {
- List<String> eventTypeNames = new ArrayList<>(Arrays.asList(listenEventTypeStrings));
+ List<String> triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries));
- if (eventTypeNames.isEmpty()) {
+ if (triggerConditions.isEmpty()) {
throw new IllegalArgumentException(
"Action "
+ actionName
- + " must specify at least one event type via listenEventTypes.");
+ + " must specify at least one trigger entry via @Action(value = ...).");
}
// Create an Action
- Action action = new Action(actionName, function, eventTypeNames, config);
+ Action action = new Action(actionName, function, triggerConditions, config);
// Add to actions map
actions.put(action.getName(), action);
// Add to actionsByEvent map
- for (String eventTypeName : eventTypeNames) {
+ for (String eventTypeName : triggerConditions) {
Review Comment:
This site builds the routing map from the raw `triggerConditions`, but the built-in path just below (`addBuiltAction`, L217) builds it through `action.getListenEventTypes()` — and that accessor's javadoc says a follow-up overrides it to "filter out non-type entries" once CEL lands. Python's `agent_plan.py:148` also reads the raw field. So of the three routing sites, only the built-in one — which never carries CEL — goes through the filtering seam; the two that handle user-supplied conditions bypass it.
Today they're equivalent, since every entry is a plain event-type name. But once CEL expressions enter `trigger_conditions`, these two sites would register strings like `"type == EventType.ChatResponseEvent && retryCount > 0"` as literal keys in `actionsByEvent` — exactly what the accessor exists to strip. Would it make sense to route all three through the accessor now, or to defer the accessor until the CEL PR and read the raw field everywhere in the meantime? Either way the three sites agreeing seems worth locking in. Not a blocker for this PR — flagging while the context is fresh.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 merged PR #756:
URL: https://github.com/apache/flink-agents/pull/756
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on PR #756:
URL: https://github.com/apache/flink-agents/pull/756#issuecomment-4687049436
**YAML `trigger_conditions`: should CEL also accept YAML event aliases?**
I have a local implementation for the YAML API path, but there is one syntax question worth aligning on. When the condition-expression support was originally designed, the YAML API path had not been added yet, so the interaction between YAML aliases and CEL expressions was not considered at that time.
Today the YAML loader already supports short aliases for built-in event types. For example:
```yaml
trigger_conditions:
- input
```
is resolved by the YAML layer to the built-in event type string before it reaches `plan.Action`.
For CEL conditions, the current Java/Python condition implementation uses the `EventType.X` namespace instead:
```yaml
trigger_conditions:
- >-
type == EventType.ChatResponseEvent
&& priority == 'high'
```
This means a mixed list may contain two notations for built-in event types:
```yaml
actions:
- name: handle_priority_input
type: java
function: com.example.MyActions:handlePriorityInput
trigger_conditions:
- input
- >-
type == EventType.ChatResponseEvent
&& priority == 'high'
```
So `input` and `EventType.InputEvent` refer to the same conceptual event type, but they appear in different syntactic forms.
I also considered allowing CEL expressions to use YAML short aliases directly. However, I do not think we should add this. Alias names such as `input`, `output`, and `chat_response` are common words that users may also use as event attribute names. If we inject them as top-level CEL variables, they could conflict with user-defined fields and make the expression semantics less clear.
So my preference is to keep the boundary simple:
* short aliases are only supported by the YAML loader for single-token type matches;
* CEL expressions should continue to use `EventType.X` for built-in event types.
Does this trade-off look acceptable for the YAML API, or do we want a different shape before finalizing it? The main alternatives I see are either standardizing YAML entirely on `EventType.X` as well, which would be a breaking change for existing YAMLs, or splitting plain type matches and CEL filters into separate YAML fields, although that would lose the unified per-condition list shape discussed earlier.
Concrete feedback welcome — particularly from anyone who has written non-trivial YAML agents and has an opinion on which form they reach for first.
/cc @wenjin272
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3361312092
##########
plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java:
##########
@@ -66,10 +66,15 @@ public Action deserialize(JsonParser jsonParser, DeserializationContext deserial
throw new IOException("Unsupported function type: " + funcType);
}
- // Deserialize listenEventTypes
- List<String> listenEventTypes = new ArrayList<>();
- node.get("listen_event_types")
- .forEach(eventTypeNode -> listenEventTypes.add(eventTypeNode.asText()));
+ // Deserialize trigger_conditions (fall back to legacy listen_event_types for older JSONs)
+ List<String> triggerConditions = new ArrayList<>();
+ JsonNode triggerNode = node.get("trigger_conditions");
+ if (triggerNode == null) {
+ triggerNode = node.get("listen_event_types");
Review Comment:
Good catch — you're right that the regenerated snapshot fixtures mean the legacy branch is never exercised under test. I'll add a focused deserialization test that feeds a JSON blob with the old listen_event_types key and asserts it maps correctly to getTriggerConditions().
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3392856213
##########
python/flink_agents/api/events/event_type.py:
##########
@@ -0,0 +1,66 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#################################################################################
+"""Built-in event-type constants, sourced from each ``XxxEvent.EVENT_TYPE``."""
+
+from __future__ import annotations
+
+from flink_agents.api.events.chat_event import (
+ ChatRequestEvent as _ChatRequestEvent,
+)
+from flink_agents.api.events.chat_event import (
+ ChatResponseEvent as _ChatResponseEvent,
+)
+from flink_agents.api.events.context_retrieval_event import (
+ ContextRetrievalRequestEvent as _ContextRetrievalRequestEvent,
+)
+from flink_agents.api.events.context_retrieval_event import (
+ ContextRetrievalResponseEvent as _ContextRetrievalResponseEvent,
+)
+from flink_agents.api.events.event import (
+ InputEvent as _InputEvent,
+)
+from flink_agents.api.events.event import (
+ OutputEvent as _OutputEvent,
+)
+from flink_agents.api.events.tool_event import (
+ ToolRequestEvent as _ToolRequestEvent,
+)
+from flink_agents.api.events.tool_event import (
+ ToolResponseEvent as _ToolResponseEvent,
+)
+
+
+class EventType:
+ """Namespace of built-in event-type constants.
+
+ Usage: ``@action(EventType.InputEvent)``.
+ """
+
+ InputEvent: str = _InputEvent.EVENT_TYPE
+ OutputEvent: str = _OutputEvent.EVENT_TYPE
+ ChatRequestEvent: str = _ChatRequestEvent.EVENT_TYPE
+ ChatResponseEvent: str = _ChatResponseEvent.EVENT_TYPE
+ ToolRequestEvent: str = _ToolRequestEvent.EVENT_TYPE
+ ToolResponseEvent: str = _ToolResponseEvent.EVENT_TYPE
+ ContextRetrievalRequestEvent: str = _ContextRetrievalRequestEvent.EVENT_TYPE
+ ContextRetrievalResponseEvent: str = _ContextRetrievalResponseEvent.EVENT_TYPE
+
+ def __init__(self) -> None:
Review Comment:
I think the __init__ method is unnecessary. We don’t have this design in `ResourceName` either.
##########
api/src/main/java/org/apache/flink/agents/api/annotation/Action.java:
##########
@@ -59,12 +60,8 @@
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface Action {
- /**
- * List of event type strings that this action should respond to.
- *
- * @return Array of event type strings
- */
- String[] listenEventTypes();
+ /** Event type name strings; multiple entries have OR semantics. */
+ String[] value();
Review Comment:
Here, we use `value` as the parameter name, whereas Python’s `Action` decorator uses `trigger_conditions`.
I understand this discrepancy arises because in Java annotations, only a member named `value` can be used without explicitly specifying the parameter name during declaration.
Therefore, to improve usability for users, I believe this inconsistency is acceptable. However, we should probably explain this in the comments.
##########
plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java:
##########
@@ -66,10 +66,15 @@ public Action deserialize(JsonParser jsonParser, DeserializationContext deserial
throw new IOException("Unsupported function type: " + funcType);
}
- // Deserialize listenEventTypes
- List<String> listenEventTypes = new ArrayList<>();
- node.get("listen_event_types")
- .forEach(eventTypeNode -> listenEventTypes.add(eventTypeNode.asText()));
+ // Deserialize trigger_conditions (fall back to legacy listen_event_types for older JSONs)
+ List<String> triggerConditions = new ArrayList<>();
+ JsonNode triggerNode = node.get("trigger_conditions");
+ if (triggerNode == null) {
+ triggerNode = node.get("listen_event_types");
Review Comment:
Although both @weiqingy and @wzhero1 agree on handling backward compatibility, I believe it is unnecessary at this stage. Flink-Agents is still in the 0.x series, where API compatibility is not guaranteed. In fact, we have already introduced some breaking API changes in version 0.3 (e.g., #631), so maintaining backward compatibility specifically for listen_event_types offers limited value.
We will begin preparing for API stability and compatibility commitments starting with the next release (0.4), and formally commit to backward compatibility from version 1.0 onwards. Until then, I consider keeping the codebase clean and simple to be a higher priority.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "rosemarYuan (via GitHub)" <gi...@apache.org>.
rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3394001395
##########
python/flink_agents/api/events/event_type.py:
##########
@@ -0,0 +1,66 @@
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#################################################################################
+"""Built-in event-type constants, sourced from each ``XxxEvent.EVENT_TYPE``."""
+
+from __future__ import annotations
+
+from flink_agents.api.events.chat_event import (
+ ChatRequestEvent as _ChatRequestEvent,
+)
+from flink_agents.api.events.chat_event import (
+ ChatResponseEvent as _ChatResponseEvent,
+)
+from flink_agents.api.events.context_retrieval_event import (
+ ContextRetrievalRequestEvent as _ContextRetrievalRequestEvent,
+)
+from flink_agents.api.events.context_retrieval_event import (
+ ContextRetrievalResponseEvent as _ContextRetrievalResponseEvent,
+)
+from flink_agents.api.events.event import (
+ InputEvent as _InputEvent,
+)
+from flink_agents.api.events.event import (
+ OutputEvent as _OutputEvent,
+)
+from flink_agents.api.events.tool_event import (
+ ToolRequestEvent as _ToolRequestEvent,
+)
+from flink_agents.api.events.tool_event import (
+ ToolResponseEvent as _ToolResponseEvent,
+)
+
+
+class EventType:
+ """Namespace of built-in event-type constants.
+
+ Usage: ``@action(EventType.InputEvent)``.
+ """
+
+ InputEvent: str = _InputEvent.EVENT_TYPE
+ OutputEvent: str = _OutputEvent.EVENT_TYPE
+ ChatRequestEvent: str = _ChatRequestEvent.EVENT_TYPE
+ ChatResponseEvent: str = _ChatResponseEvent.EVENT_TYPE
+ ToolRequestEvent: str = _ToolRequestEvent.EVENT_TYPE
+ ToolResponseEvent: str = _ToolResponseEvent.EVENT_TYPE
+ ContextRetrievalRequestEvent: str = _ContextRetrievalRequestEvent.EVENT_TYPE
+ ContextRetrievalResponseEvent: str = _ContextRetrievalResponseEvent.EVENT_TYPE
+
+ def __init__(self) -> None:
Review Comment:
Thanks for your suggestions regarding the review. Dropped the `__init__` guard. Agreed it adds little value here, so removing it aligns with that idiom.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [api][java][python] Introduce EventType constants and unify Action trigger entry [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3496805996
##########
plan/src/test/resources/actions/action_java_function.json:
##########
@@ -6,5 +6,5 @@
"method_name": "legal",
"parameter_types": ["org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext"]
},
- "listen_event_types": ["_input_event"]
+ "trigger_conditions": ["_input_event"]
Review Comment:
The field name in `action_invalid_event_type.json`, `action_missing_fields.json` and `action_invalid_function_type.json` were not adapted.
##########
plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java:
##########
@@ -66,10 +66,12 @@ public Action deserialize(JsonParser jsonParser, DeserializationContext deserial
throw new IOException("Unsupported function type: " + funcType);
}
- // Deserialize listenEventTypes
- List<String> listenEventTypes = new ArrayList<>();
- node.get("listen_event_types")
- .forEach(eventTypeNode -> listenEventTypes.add(eventTypeNode.asText()));
+ // Deserialize trigger_conditions
+ List<String> triggerConditions = new ArrayList<>();
+ JsonNode triggerNode = node.get("trigger_conditions");
+ if (triggerNode != null) {
Review Comment:
The `trigger_conditions` should not be null.
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -184,27 +184,27 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
private void extractActions(
String actionName,
- String[] listenEventTypeStrings,
+ String[] triggerEntries,
org.apache.flink.agents.plan.Function function,
Map<String, Object> config)
throws Exception {
- List<String> eventTypeNames = new ArrayList<>(Arrays.asList(listenEventTypeStrings));
+ List<String> triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries));
- if (eventTypeNames.isEmpty()) {
+ if (triggerConditions.isEmpty()) {
throw new IllegalArgumentException(
"Action "
+ actionName
- + " must specify at least one event type via listenEventTypes.");
+ + " must specify at least one trigger entry via @Action(value = ...).");
Review Comment:
Consider using `@Action(EventType.x)` in the error message.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org