weiqingy commented on PR #546: URL: https://github.com/apache/flink-agents/pull/546#issuecomment-4341130301
Thanks for reviewing, Xintong. I've updated the PR (commits `300a0fa..e2e4e03`) to address all the comments. High-level summary: - **Comment #3** — `jobIdentifier` should live on the operator: moved the field and its `identifier_state` initialization out of `OperatorStateManager` and onto `ActionExecutionOperator`. Savepoint compatibility preserved (state descriptor name unchanged). → `1139b9c` - **Comment #4** — `@Nullable` / `Optional` for nullable returns: went with `@Nullable` for consistency with the existing `@Nullable ActionStateStore` style. Applied across all 5 managers (16 methods total). → `85df684` - **Comment #5** — `inputIsJava` never used: removed the field; the constructor parameter is forwarded directly to `EventRouter`. → `1139b9c` - **Comments #6 + #7** — the 2 maps belong on `ActionTaskContextManager` and the current placement violates the "no manager-to-manager references" goal: moved `continuationContexts` and `pythonAwaitableRefs` (and their accessors) to `ActionTaskContextManager`. The third map, `actionTaskDurableContexts`, stays in `DurableExecutionManager` since DEM consumes it directly via `setupDurableExecutionContext`. As a side effect, `createAndSetRunnerContext` no longer needs the DEM parameter. The only remaining cross-manager touchpoint is `transferContexts(...)` taking DEM as a method parameter → `ab0c581` - **Top-level — Javadocs**: expanded class-level Javadoc on each of the 5 managers to spell out the contract (responsibilities, owned state, lifecycle, what each does and does not own, and the no-manager-to-manager-references constraint). → `6991b99` - **Top-level — unit tests**: went with a hybrid scope to keep the PR reviewable — added focused contract tests per manager (4 new test files, 7 tests) covering the core promise of each component (DEM no-store no-op + persist round-trip, ATCM per-task map isolation + Python-context guard, EventRouter Java passthrough + dispatch by event class, PythonBridge no-Python early-return). Heavier coverage (Flink-harness `OperatorStateManager` tests, full snapshot/recovery, watermark draining, listener notification) tracked as a follow-up. Happy to expand inline if you'd prefer. → `e2e4e03` PTAL when you have a moment. -- 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]
