aglinxinyuan opened a new pull request, #7794: URL: https://github.com/apache/texera/pull/7794
### What changes were proposed in this PR? Removes `AsyncRPCClient.get_worker_interface` and the `_create_proxy` helper it is the sole caller of. **109 lines deleted, 0 added.** Python workers talk only to the coordinator. The one production construction of the Python `AsyncRPCClient` is `main_loop.py:114`, and it uses `coordinator_stub()` (lines 271, 651, 671, 817) and `receive` (line 475). Worker-to-worker RPC initiated from Python is unused. | Grep | Result | |---|---| | `get_worker_interface` | its definition, plus three call sites — all in `test_async_rpc_client.py` | | `WorkerServiceStub` | `async_rpc_client.py` only: the import, the return annotation, one argument. No `.scala` hit | | `_create_proxy` | its definition, and one call inside `get_worker_interface` — its sole caller | | `_stream_unary` / `_unary_stream` / `_stream_stream` | only the `Proxy` blockers inside `_create_proxy`, plus those three tests | There is no `getattr`-style dynamic access anywhere. Scala's `workerInterface` (`AsyncRPCClient.scala:140`, `WorkerServiceFs2Grpc`) is a separate class and unrelated to this Python method. `_create_proxy` is removed whole rather than just its `_unary_unary` body: with its only caller gone, leaving a `Proxy` class holding three stream blockers and no reachable constructor would just relocate the dead code. The now-orphaned `WorkerServiceStub` import goes too — it was the only orphan, since every other import is still used by `_assign_context` / `_create_future` / `receive`. ### Three tests were removed, deliberately `test_stream_unary_blocked`, `test_unary_stream_blocked` and `test_stream_stream_blocked` each begin with `client.get_worker_interface(...)` and then assert on blockers that exist only inside `_create_proxy`'s local `Proxy`. They exercise nothing else. Their removal orphaned `import pytest` — the file's only `pytest.` uses were those three `pytest.raises` calls — so that went as well. The coordinator path is untouched: it goes through `_assign_context`, never had the blockers, and keeps its own tests. ### Scope: one item was assessed and deliberately kept The audit that produced this also flagged `IcebergDocument`'s read side (`get_uri`, `get_range`, `get_after`, `get_count`) as having no production callers — only `get()` and `writer()` are used (`input_port_materialization_reader_runnable.py:161,170`, `main_loop.py:157,216`, `output_manager.py:148`). **None of it is removed here**, because unlike the RPC proxy these are working implementations of `@abstractmethod`s on `ReadonlyVirtualDocument`, i.e. the read contract `IcebergDocument` exists to fulfil: - `get_range`, `get_after` and `get_count` have live passing tests asserting real Iceberg behaviour, and `get_range`/`get_after` are the only public entry points to the ranged form of `_get_using_file_sequence_order`. Removing them would strand `num_of_skipped_records`, `total_records_to_return`, `_skip_records` and the skip loop in `IcebergIterator`. - `get_uri` has zero references of any kind, so it *is* removable — but it is a working override, and deleting it silently changes behaviour from "returns the table location" to "raises `NotImplementedError`" via `VirtualDocument`'s fallback, and drops parity with Scala's `IcebergDocument.getURI`. That is an API decision, not a cleanup, so it is left for a maintainer. ### Verification | Run | Result | |---|---| | `core/architecture/rpc/` | **38 passed** | | `core/architecture/rpc/` + `core/storage/model/` | 49 passed | | `core/architecture` + `core/storage` + `test_main_loop.py`, `-m "not integration"` | 448 passed, 1 deselected | The deselected test is `test_iceberg_rest_catalog_integration.py`, excluded by its own `pytestmark`. `ruff format --check src/main/python src/test/python` → exit 0, "209 files already formatted". `ruff check` → exit 0, "All checks passed!". `git diff --numstat` shows 0 insertions against 109 deletions, so there are no incidental rewrites. ### Any related issues, documentation, discussions? Closes #7793 ### How was this PR tested? ``` python -m pytest src/test/python/core/architecture/rpc/ -q ``` ``` 38 passed, 1 warning ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) -- 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]
