da-daken commented on PR #657: URL: https://github.com/apache/flink-agents/pull/657#issuecomment-4556065743
> Thanks for picking this up — a few cross-cutting questions before this lands: > > 1. **Python mirror is missing.** `python/flink_agents/api/core_options.py` mirrors every entry in the Java `AgentExecutionOptions` today (`MAX_RETRIES`, `RETRY_WAIT_INTERVAL`, the async flags, etc.). The three new short-term-memory TTL options are not added on the Python side, so a Python user can't reach them via the typed API — they'd have to fall back to raw string keys. Could we add the mirrors (and a matching test) in this PR? CLAUDE.md is explicit about Java↔Python parity for shared options. > 2. **Are we comfortable exposing `StateTtlConfig.UpdateType` and `StateTtlConfig.StateVisibility` directly in the public API?** Two of the new options take Flink internal enums as their value type, in a class under `org.apache.flink.agents.api.agents`. That couples the public surface to Flink's state API forever, and makes the Python mirror painful (we'd need to redefine the enums on the Python side or fall back to strings). One alternative is to wrap them in our own enums (`ShortTermMemoryTtlUpdate` / `ShortTermMemoryTtlVisibility`) with one-to-one mappings — costs a few lines and a translation step, buys API independence. What do you think? @weiqingy Thanks for your review. You're right, I submitted the new code. 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
