mengw15 opened a new pull request, #8105:
URL: https://github.com/apache/texera/pull/8105
### What changes were proposed in this PR?
Adds `PythonWorkflowWorkerSpec` — the package had specs for the proxy
client/server,
the startup config and the batch queue, but none for the worker actor
itself. 12 tests
covering the interpreter selection, the path/config vals, the teardown
sequence, and
the actor's own message handlers.
- **`choosePythonBin`** — all four outcomes, one input value apart: no
`cuid`, a `cuid`
with a whitespace-only environment name (the name is trimmed before the
emptiness
check), a `cuid` and a name whose interpreter is not on disk, and a name
whose
interpreter is. The last one creates a real executable under the venv root
and skips
with `assume` if the filesystem ignores the executable bit, so it can
never pass
vacuously.
- **`pythonSrcDirectory`** — asserted by path tail (`src/main/python`)
rather than as an
absolute string, so the test does not depend on where the repo is checked
out.
**`RENVPath`** — matches `UdfConfig.rPath.trim`.
- **`postStop`** — both arms. With the port promise released and a stand-in
process
installed, stopping the actor closes the proxy client, shuts both
executors down and
destroys the process; with a process whose `destroy()` throws, the actor
still
terminates and the steps that ran before the failure still took effect, so
the failure
is proven not to escape.
- **`handleInputMessage`** — a `ControlInvocation` is handed to the Python
side and the
message is acknowledged with the right id, in-memory size and queued
credit (which
also exercises `getQueuedCredit`). **`receiveCreditMessages`** — a
`CreditRequest` is
answered with a `CreditResponse` for the same channel.
**`handleBackpressure`** — the
command is queued for the Python process, and the actor keeps serving its
protocol
afterwards.
- **`loadFromCheckpoint`** — throws `NotImplementedError`.
**Two things the issue's plan did not anticipate, both worth knowing.**
The worker offers no injection seam. `initState` starts a proxy server that
retries a
socket bind until it succeeds, spawns a real interpreter, and starts a
Flight client;
`pythonProxyClient`, `pythonServerProcess` and the executors are all
`private`. So the
suite subclasses the worker and overrides `initState` to a no-op — an
ordinary override,
and the only way to get a live instance without any of that — and reads the
`private`
members by reflection, which is already how a dozen specs in this module
reach internals.
More importantly, **`postStop` cannot run at all on a worker whose proxy
server never
started**: its first statement forces `pythonProxyClient`, whose constructor
blocks in
`Await.result(portNumberPromise)` with no timeout. The suite therefore
fulfils that
promise before stopping the actor. Without it the test hangs rather than
fails, which is
worth remembering for anything else that touches this actor's teardown.
Out of scope, deliberately: `startPythonProcess` (runs a real interpreter,
not guaranteed
present on a runner) and `startProxyServer` (retries a socket bind in a
loop).
No production code was changed.
### Any related issues, documentation, discussions?
Closes #8103.
### How was this PR tested?
`sbt "WorkflowExecutionService/testOnly *PythonWorkflowWorkerSpec"` — 12
passed, repeated
3× for stability; the whole `pythonworker` package stays green (51 tests, 39
pre-existing).
`WorkflowExecutionService/Test/scalafmtCheck` and `Test/scalafix --check`
clean. Failure
path verified by breaking one assertion in each of the 12 tests: 12 failed /
0 passed,
non-zero exit, then restored to green.
A local per-spec Jacoco figure is not available — this build exposes no
filtered Jacoco
task, and the full `WorkflowExecutionService/jacoco` runs the entire engine
suite — so the
coverage delta is left to the Codecov report on this PR rather than quoted
here.
Determinism: no interpreter is launched and no socket is opened; the process
handle is a
recording stand-in; the venv fixture is created under a cuid no other suite
writes to and
removed in `afterAll`, which also shuts the actor system down so the suite
leaks no
threads; every wait is an `expectMsg`/`expectTerminated` deadline rather
than a sleep.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
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]
