wzhero1 commented on issue #770:
URL: https://github.com/apache/flink-agents/issues/770#issuecomment-4629339658
Thanks @luanxu-dev. The silent-fallback point is well taken, but I checked
the history and
`timeoutSeconds` isn't a legacy key we need to keep:
- Released 0.2.0 and 0.2.1 both read `descriptor.getArgument("timeout")`,
and the docs have
always shown `addInitialArgument("timeout", N)`. `timeout` is the
documented, released,
working key.
- The `getArgument("timeoutSeconds")` read was introduced by the MCP retry
refactor (#531)
after 0.2.1 and has only ever existed on `main` (0.3.0-dev) — it silently
broke the
documented `timeout` key. It's a regression, not a key any released
version exposed.
So `timeoutSeconds` never took effect as a descriptor argument, and
recognizing it (even
deprecated) would mean inventing an alias that never existed for users. The
fix in #771
restores `timeout` and unifies the serialized JSON key to `timeout` as well,
so the docs, the
Java descriptor/JSON, and the Python `timeout` field all line up.
On silent fallback in general: agreed it's easy to misdiagnose, but it's
framework-wide — every
resource reads its descriptor args via `getArgument(...)`, which returns
null → default for any
unrecognized key. I'd rather add descriptor-key validation / "unknown key"
warnings as a
separate framework-level change than special-case MCP timeout here. Happy to
file a follow-up
if you think it's worth it.
--
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]
