-
Notifications
You must be signed in to change notification settings - Fork 140
[api][java][python] Introduce EventType constants and unify Action trigger entry #756
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
Changes from all commits
4b489e3
ba5b385
d90e7cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /* | ||
| * 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; | ||
|
|
||
| /** | ||
| * Compile-time constants for built-in event types, sourced from each {@code XxxEvent.EVENT_TYPE}. | ||
| * | ||
| * <p>Usage: {@code @Action(EventType.InputEvent)}. | ||
| */ | ||
| public final class EventType { | ||
|
|
||
| public static final String InputEvent = org.apache.flink.agents.api.InputEvent.EVENT_TYPE; | ||
| public static final String OutputEvent = org.apache.flink.agents.api.OutputEvent.EVENT_TYPE; | ||
| public static final String ChatRequestEvent = | ||
| org.apache.flink.agents.api.event.ChatRequestEvent.EVENT_TYPE; | ||
| public static final String ChatResponseEvent = | ||
| org.apache.flink.agents.api.event.ChatResponseEvent.EVENT_TYPE; | ||
| public static final String ToolRequestEvent = | ||
| org.apache.flink.agents.api.event.ToolRequestEvent.EVENT_TYPE; | ||
| public static final String ToolResponseEvent = | ||
| org.apache.flink.agents.api.event.ToolResponseEvent.EVENT_TYPE; | ||
| public static final String ContextRetrievalRequestEvent = | ||
| org.apache.flink.agents.api.event.ContextRetrievalRequestEvent.EVENT_TYPE; | ||
| public static final String ContextRetrievalResponseEvent = | ||
| org.apache.flink.agents.api.event.ContextRetrievalResponseEvent.EVENT_TYPE; | ||
|
|
||
| private EventType() {} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,23 +24,24 @@ | |
| import java.lang.annotation.Target; | ||
|
|
||
| /** | ||
| * Annotation for marking a method as an agent action. | ||
| * Marks a method as an agent action triggered by matching events. | ||
| * | ||
| * <p>This annotation specifies which event types the action should respond to. The annotated method | ||
| * will be triggered when any of the specified event types occur. | ||
| * <p>Each {@link #value()} entry is an event type name string. Use the {@code EVENT_TYPE} constants | ||
| * on built-in event classes, the {@link org.apache.flink.agents.api.EventType} constants, or plain | ||
| * strings for custom events. Multiple entries combine with OR. | ||
| * | ||
| * <p>Events are specified as type strings via {@link #listenEventTypes()}. Use the {@code | ||
| * EVENT_TYPE} constants on built-in event classes for standard events, or plain strings for custom | ||
| * events. | ||
| * <pre>{@code | ||
| * // Built-in event type via the EventType constant | ||
| * @Action(EventType.InputEvent) | ||
| * | ||
| * <p>Example usage: | ||
| * // Equivalent via the legacy class constant | ||
| * @Action(InputEvent.EVENT_TYPE) | ||
| * | ||
| * <pre>{@code | ||
| * @Action(listenEventTypes = {InputEvent.EVENT_TYPE}) | ||
| * public void handleInput(Event event, RunnerContext ctx) { ... } | ||
| * // User-defined event type | ||
| * @Action("MyCustomEvent") | ||
| * | ||
| * @Action(listenEventTypes = {InputEvent.EVENT_TYPE, "MyCustomEvent"}) | ||
| * public void handleMultiple(Event event, RunnerContext ctx) { ... } | ||
| * // Multiple types (OR semantics) | ||
| * @Action({EventType.InputEvent, "MyCustomEvent"}) | ||
| * }</pre> | ||
| * | ||
| * <p>For a cross-language action, set {@link #target()} to a {@link PythonFunction} with a | ||
|
|
@@ -49,7 +50,7 @@ | |
| * | ||
| * <pre>{@code | ||
| * @Action( | ||
| * listenEventTypes = {InputEvent.EVENT_TYPE}, | ||
| * value = EventType.InputEvent, | ||
| * target = @PythonFunction(module = "my_pkg.handlers", qualname = "handle_input")) | ||
| * public void handleInput(Event event, RunnerContext ctx) { | ||
| * throw new UnsupportedOperationException("cross-language stub"); | ||
|
|
@@ -60,11 +61,11 @@ | |
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface Action { | ||
| /** | ||
| * List of event type strings that this action should respond to. | ||
| * | ||
| * @return Array of event type strings | ||
| * Event type name strings; multiple entries have OR semantics. Named {@code value} (not {@code | ||
| * triggerConditions}) to enable the {@code @Action({...})} shorthand (JLS §9.7.3); corresponds | ||
| * to Python's {@code *trigger_conditions}. | ||
| */ | ||
| String[] listenEventTypes(); | ||
| String[] value(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, we use I understand this discrepancy arises because in Java annotations, only a member named Therefore, to improve usability for users, I believe this inconsistency is acceptable. However, we should probably explain this in the comments.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point — added a one-sentence note on explaining:
|
||
|
|
||
| /** | ||
| * Cross-language target. When {@link PythonFunction#module()} is non-empty, dispatch routes to | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| * 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.apache.flink.agents.api.event.ChatRequestEvent; | ||
| import org.apache.flink.agents.api.event.ChatResponseEvent; | ||
| import org.apache.flink.agents.api.event.ContextRetrievalRequestEvent; | ||
| import org.apache.flink.agents.api.event.ContextRetrievalResponseEvent; | ||
| import org.apache.flink.agents.api.event.ToolRequestEvent; | ||
| import org.apache.flink.agents.api.event.ToolResponseEvent; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| /** Tests for {@link EventType}. */ | ||
| class EventTypeTest { | ||
|
|
||
| @Test | ||
| void builtInConstantsMatchEventClassConstants() { | ||
| assertEquals(InputEvent.EVENT_TYPE, EventType.InputEvent); | ||
| assertEquals(OutputEvent.EVENT_TYPE, EventType.OutputEvent); | ||
| assertEquals(ChatRequestEvent.EVENT_TYPE, EventType.ChatRequestEvent); | ||
| assertEquals(ChatResponseEvent.EVENT_TYPE, EventType.ChatResponseEvent); | ||
| assertEquals(ToolRequestEvent.EVENT_TYPE, EventType.ToolRequestEvent); | ||
| assertEquals(ToolResponseEvent.EVENT_TYPE, EventType.ToolResponseEvent); | ||
| assertEquals( | ||
| ContextRetrievalRequestEvent.EVENT_TYPE, EventType.ContextRetrievalRequestEvent); | ||
| assertEquals( | ||
| ContextRetrievalResponseEvent.EVENT_TYPE, EventType.ContextRetrievalResponseEvent); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconsider the
EventTypeaggregation classEventType.InputEventduplicatesInputEvent.EVENT_TYPE— two sources of truth for the same value. Theregister()/lookup()/lookupOrSelf()registry has zero callers in production code today.InputEvent.EVENT_TYPEis 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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.