You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "CodeTrainerMan (via GitHub)" <gi...@apache.org> on 2026/09/24 20:16:35 UTC
[PR] [api][java][python] Reject non-positive max_results in ContextRetrievalRequestEvent [flink-agents]
CodeTrainerMan opened a new pull request, #1157:
URL: https://github.com/apache/flink-agents/pull/1157
## What is the purpose of the change
`ContextRetrievalRequestEvent` accepted a non-positive `max_results` and stored it as-is, on
both the Java and the Python side. The value is passed straight into `VectorStoreQuery` by
`ContextRetrievalAction`, so after #1144 an illegal value only fails **inside the action at
runtime**, far away from where the user actually built the event. The event is also serialized
and exchanged between the Java and Python runtimes, so an invalid value can cross the language
boundary before it blows up.
This PR rejects `max_results <= 0` at construction time, so the failure points at the
parameter that is actually wrong.
Fixes #1156
## Brief change log
* `ContextRetrievalRequestEvent` (Java): reject a non-positive `max_results` with an
`IllegalArgumentException` naming the parameter and its value, consistent with #1144
* `ContextRetrievalRequestEvent` (Python): reject a non-positive `max_results` with a
`ValueError`
* Added `ContextRetrievalRequestEventTest` (Java) and `test_context_retrieval_event.py`
(Python)
## Verifying this change
* Java: `mvn -pl api test -Dtest=ContextRetrievalRequestEventTest` passes (4 tests)
* Java format: `mvn -pl api spotless:apply`
* Python: `ruff format` and `ruff check` are clean; the Python unit tests require `pyflink`,
which is not installed locally, so they are covered by CI
## Does this pull request potentially affect one of the following parts
* Dependencies (does it add or upgrade a dependency): no
* The public API: no signature change, but constructing the event with `max_results <= 0` now
fails fast instead of failing later inside the action
--
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] Reject non-positive max_results in ContextRetrievalRequestEvent [flink-agents]
Posted by "CodeTrainerMan (via GitHub)" <gi...@apache.org>.
CodeTrainerMan commented on PR #1157:
URL: https://github.com/apache/flink-agents/pull/1157#issuecomment-5878636795
Thanks for the suggestion, @wenjin272 — agreed, the Java reconstruction paths should behave the same as direct construction.
I've moved the check into a shared `validateMaxResults` helper and now call it from the `@JsonCreator (id, attributes)` constructor as well. Since `Event.reconstructFrom` builds instances through that constructor, both reconstruction paths (`fromEvent` and JSON deserialization) are covered in one place. The exception type and message are unchanged, so both languages keep reporting identical errors for the same input.
Added four tests to `ContextRetrievalRequestEventTest`: reconstruction from attributes, reconstruction via `fromEvent`, the valid case, and a missing `max_results` attribute (not rejected, to stay compatible with payloads that don't set it). The full `api` module suite passes locally (488 tests).
If anything still comes up after this push, I'll dig into it and follow up with another commit.
--
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] Reject non-positive max_results in ContextRetrievalRequestEvent [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #1157:
URL: https://github.com/apache/flink-agents/pull/1157#discussion_r4119321629
##########
api/src/main/java/org/apache/flink/agents/api/event/ContextRetrievalRequestEvent.java:
##########
@@ -37,8 +37,20 @@ public ContextRetrievalRequestEvent(String query, String vectorStore) {
this(query, vectorStore, DEFAULT_MAX_RESULTS);
}
+ /**
+ * Creates a new context retrieval request event.
+ *
+ * @param query the search query text
+ * @param vectorStore the name of the vector store to query
+ * @param maxResults the maximum number of results to return; must be positive
+ * @throws IllegalArgumentException if {@code maxResults} is non-positive
+ */
public ContextRetrievalRequestEvent(String query, String vectorStore, int maxResults) {
super(EVENT_TYPE);
+ if (maxResults <= 0) {
Review Comment:
Could we apply the same validation in the `UUID`/`attributes` constructor? Java’s `fromEvent()` and JSON deserialization use that constructor, so `max_results <= 0` still passes reconstruction and fails later in `VectorStoreQuery`. Python’s `from_event()` already goes through the validated constructor. Please add regression tests for both Java reconstruction paths.
--
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] Reject non-positive max_results in ContextRetrievalRequestEvent [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 merged PR #1157:
URL: https://github.com/apache/flink-agents/pull/1157
--
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