eemario commented on code in PR #1320:
URL: https://github.com/apache/flink-agents/pull/1320#discussion_r4356823745
##########
api/src/main/java/org/apache/flink/agents/api/subagent/SubagentFuture.java:
##########
@@ -44,8 +44,15 @@ public String getCallId() {
public abstract boolean isDone();
/**
- * Resolves the invocation, waiting until it reaches a terminal state.
Failures converge into a
- * failed {@link SubagentResult} rather than a separately reported
exceptional completion.
+ * Resolves the invocation, waiting until it reaches a terminal state. A
comprehensible
+ * sub-agent failure is returned as a failed {@link SubagentResult};
system-level exceptions,
+ * cancellation and interruption propagate to the caller.
+ *
+ * <p>Do not retry an await that throws. Any retry belongs inside its
await implementation.
+ * Successful repeated awaits may return the cached result. The built-in
deferred and async
Review Comment:
One gap worth noting on internal futures: child failures fold into a failed
SubagentResult and resolve the handle, so those are fine. Framework-side faults
(bootstrap, SubgraphExecution await/finish) still follow the pre-fix path
though — ToolCallAction absorbs the exception and the action then dies at
finalization on the unresolved handle, instead of failing directly. Do you
think it should be fixed in this PR, or tracked as a follow-up?
##########
api/src/main/java/org/apache/flink/agents/api/subagent/SubagentFuture.java:
##########
@@ -44,8 +44,15 @@ public String getCallId() {
public abstract boolean isDone();
/**
- * Resolves the invocation, waiting until it reaches a terminal state.
Failures converge into a
- * failed {@link SubagentResult} rather than a separately reported
exceptional completion.
+ * Resolves the invocation, waiting until it reaches a terminal state. A
comprehensible
Review Comment:
One question on the group path scoped out in the description: when a caller
calls awaitAll() and catches a failure (say a prepare failure), every handle
stays pending, so finishing the action normally still fails finalization with
the unresolved-handle error. Unawaited siblings look fine per the interaction
table — but should the contract say what a catching caller is expected to do:
await the rest individually, or cancel the group?
--
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]
