[python] Remove the Python local execution path#882
Conversation
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for taking this on @weiqingy. I left one comment about doc.
Besides, LocalMemoryObject now appears to be used only by tests (test_chat_model_action, test_local_memory_object, and test_memory_reference). Since this PR removes the local execution path, could we move it out of flink_agents.runtime and keep it as a test fixture instead? That would avoid leaving a local-runtime-looking class in the production package.
There was a problem hiding this comment.
@wenjin272 Thanks for the review. Both addressed:
configuration.mdLocal-mode example: removed inb495072. It pointed at the run-without-Flink path this PR deletes. That left MiniCluster as the only special case, so I also changed the intro from "the following two cases" to the singular form.LocalMemoryObject: moved toflink_agents.runtime.tests.local_memory_objectind4243f1, so it's no longer in the production runtime package. It's now purely an in-memory test fixture, and the three importers (test_chat_model_action,test_memory_reference,test_local_memory_object) are updated — it now sits alongside its own unit test inruntime/tests.
wenjin272
left a comment
There was a problem hiding this comment.
LGTM, but there are some conflicts that need to be resolved.
Prepare removal of the Python local execution path: delete the local-path tests now covered by the remote (MiniCluster) tests added earlier, and migrate the remaining tests and examples onto the remote path. The local classes and their API are left intact here and removed in the following commit so nothing dangles. - Delete local-path tests backfilled by remote equivalents (durable execute, built-in action, get_resource, MCP, chat model, workflow memory) and the local-runner-only tests. - Migrate test_loader registration tests to construct RemoteExecutionEnvironment directly under patch (the unit lane has no built JARs, so the environment factory cannot be used there). - Migrate the RAG and quickstart examples off from_list/to_list to the remote DataStream pattern. - Drop the now-unused tool_invocations_from_events helper and its test. - Rename test_local_runner_cross_language to test_plan_java_function_routing; it now covers only PlanJavaFunction routing.
Remove the in-process local execution path now that all callers are migrated to the remote (Flink) path: - Delete local_runner.py and local_execution_environment.py. - Remove the orphaned from_list/to_list execution-environment API (abstract and remote) and get_tool_request_events. - Require a StreamExecutionEnvironment: get_execution_environment now raises when no environment is provided, instead of falling back to the local path. - Update the ReActAgent docstring example and remove the local-path sections and no-argument snippets from the documentation.
The Python local execution path is removed in this PR, so the run-without-Flink configuration example no longer applies. Drop the "Local mode" bullet and update the surrounding text, since MiniCluster is now the only special case requiring manual configuration.
With the local execution path removed, LocalMemoryObject is used only as an in-memory test fixture. Relocate it from flink_agents.runtime to flink_agents.runtime.tests so it no longer sits in the production package.
d4243f1 to
fd4b425
Compare
|
Both failing checks are unrelated to this PR — they're CI environment issues (a broken PyFlink install on one leg, an Ollama backend crash on the other), not test failures.
Neither is caused by this change: the identical e2e suite passed on the I've pushed an empty commit to re-trigger CI. |
58df8f3 to
12d1665
Compare
|
Thanks @wenjin272. I've rebased onto |
Linked issue: #829
Purpose of change
Second of two PRs retiring the Python in-process local execution path, per #829. This PR removes the local path itself.
Stacked on #881 (the additive test backfill). Until #881 merges, this PR's diff includes #881's commits; the net-new change here is the removal, in the last two commits — "Remove local-path tests and migrate remaining tests off it" and "Delete the local execution path and require a Flink environment". I'll rebase once #881 lands.
What this removes:
local_runner.pyandlocal_execution_environment.py— the pure-Python, single-process execution path selected when no Flink environment was passed.from_list/to_listtest-only API and the local-onlyget_tool_request_eventshook.get_execution_environmentnow requires aStreamExecutionEnvironmentand raises a clear error when none is provided, instead of falling back to the local path. The RAG and quickstart examples are migrated to the remote DataStream pattern.LocalMemoryObjectis kept as an in-memory test fixture (it is independent of the runner and still used by unit tests).Tests
The Python unit lane and the remote end-to-end suite pass. The local-path tests are removed because equivalent behavior is now exercised on a real Flink MiniCluster by the tests added in #881 (durable-execute sync-exception and kwargs, built-in action content and
get_resource, MCP tool and prompt invocation, workflow short-term-memory semantics, and the apply-by-name guard).API
Yes — this removes public surface: the execution-environment
from_list/to_listmethods,get_tool_request_events, and the no-argumentget_execution_environment()form (a Flink environment is now required). This is a deliberate removal of the non-production local path while the project is in beta.Documentation
doc-neededdoc-not-neededdoc-included