weiqingy commented on PR #548:
URL: https://github.com/apache/flink-agents/pull/548#issuecomment-4028880184
> @weiqingy Thanks for working on this refactor.
>
> There're a few comments from my side. Please take a look.
>
> 1. Since ·ResourceCache`and`PythonResourceBridge`are only needed in
runtime, show we move the from the`plan`package/module to`runtime`?
> 2. What are the relationship of `PythonResourceBridge` and
`PythonResourceAdapter`? Is there a clear boundary of responses? Or can they be
combined into one class?
> 3. The cross-language test in CI stuck even after a retry. This might be
related to the code changes.
Thanks for reviewing @xintongsong !
1. I kept both in plan. They only depend on plan/api types
(ResourceProvider, PythonResourceAdapter, etc.) with zero runtime imports.
ResourceCache is also used by 6 plan test files — moving it would require
either circular test dependencies or relocating tests that fundamentally test
AgentPlan behavior. Being consumed by runtime doesn't mean it should be owned
by runtime.
2. Relationship between PythonResourceBridge and PythonResourceAdapter?
They serve different roles — PythonResourceAdapter is a general-purpose
Java-Python interop interface in api, while PythonResourceBridge is a one-time
MCP server discovery utility. Combining them doesn't make sense since they're
at different abstraction levels. I renamed PythonResourceBridge →
PythonMCPResourceDiscovery to make the distinction obvious from the name.
3. Cross-language test stuck after retry.
Fixed per @wenjin272's finding — moved resourceCache.close() to the top of
ActionExecutionOperator.close(),
before pythonInterpreter.close(). Cached resources may hold Python object
references that need the
interpreter alive during cleanup. Added a comment explaining the ordering
constraint.
--
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]
