Skip to content

[python] Remove the Python local execution path#882

Open
weiqingy wants to merge 5 commits into
apache:mainfrom
weiqingy:829-pr2-remove-local-path
Open

[python] Remove the Python local execution path#882
weiqingy wants to merge 5 commits into
apache:mainfrom
weiqingy:829-pr2-remove-local-path

Conversation

@weiqingy

@weiqingy weiqingy commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

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.py and local_execution_environment.py — the pure-Python, single-process execution path selected when no Flink environment was passed.
  • The execution-environment from_list / to_list test-only API and the local-only get_tool_request_events hook.
  • The local-path tests, now covered by the remote (MiniCluster) tests added in [python] Backfill remote test coverage ahead of local execution path removal #881; and the local-path sections of the documentation.

get_execution_environment now requires a StreamExecutionEnvironment and 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. LocalMemoryObject is 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_list methods, get_tool_request_events, and the no-argument get_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-needed
  • doc-not-needed
  • doc-included

@github-actions github-actions Bot added doc-included Your PR already contains the necessary documentation updates. fixVersion/0.4.0 priority/major Default priority of the PR or issue. labels Jul 7, 2026

@wenjin272 wenjin272 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be removed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wenjin272 Thanks for the review. Both addressed:

  • configuration.md Local-mode example: removed in b495072. 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 to flink_agents.runtime.tests.local_memory_object in d4243f1, 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 in runtime/tests.

@wenjin272 wenjin272 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but there are some conflicts that need to be resolved.

weiqingy added 4 commits July 10, 2026 08:40
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.
@weiqingy weiqingy force-pushed the 829-pr2-remove-local-path branch from d4243f1 to fd4b425 Compare July 10, 2026 15:43
@weiqingy

weiqingy commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

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.

  • it-python [python-3.11] [flink-2.0]: every e2e test fails at pyflink's own Configuration() constructor with TypeError: Could not found the Java class 'org.apache.flink.configuration.Configuration' and py4j.protocol.Py4JError: org does not exist in the JVM. The Flink dist JARs were not on the JVM classpath, so it dies inside pyflink/common/configuration.py before any agent code runs — the first Flink test (mcp_test.py::test_mcp) already fails there and the rest cascade off the same broken gateway. Job.
  • it-python [python-3.12] [flink-2.3]: the LLM-driven tests fail with pemja.core.PythonException: <class 'ollama._types.ResponseError'>: llama-server process has terminated: signal: segmentation fault (core dumped). The Ollama server crashed mid-run; the non-LLM e2e tests on this same leg all passed. Job.

Neither is caused by this change: the identical e2e suite passed on the flink-1.20, flink-2.1, and flink-2.2 legs of the same run, and this PR only deletes Python local-execution code — nothing in the build, dist/, pom.xml, or the pyflink packaging.

I've pushed an empty commit to re-trigger CI.

@weiqingy weiqingy force-pushed the 829-pr2-remove-local-path branch from 58df8f3 to 12d1665 Compare July 10, 2026 16:15
@weiqingy

weiqingy commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @wenjin272. I've rebased onto main to resolve the conflict, and CI is now green. Could you take another look when you have a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-included Your PR already contains the necessary documentation updates. fixVersion/0.4.0 priority/major Default priority of the PR or issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants