aglinxinyuan opened a new pull request, #7781:
URL: https://github.com/apache/texera/pull/7781
### What changes were proposed in this PR?
Deletes `ClientActor.ClosureRequest` and its unreachable `receive` arm. Pure
deletion, no behaviour change: **−27 lines**.
`AmberClient` is the only thing that messages `ClientActor`, and it sends
exactly two things — `PoisonPill` on shutdown and `CommandRequest` for RPC
calls. A repo-wide search for `ClosureRequest` returns only the
companion-object declaration, the import pulling it into `ClientActor`, its
`receive` arm, and two tests. Nothing sends it, so the arm never runs:
```scala
case ClosureRequest(closure) => // unreachable
try sender() ! closure()
catch { case e: Throwable => sender() ! e }
```
It was a run-arbitrary-closure-on-the-actor-thread escape hatch that the
engine no longer uses.
> Reviewer note: the spec's header comment explaining why it lives in
`...engine.common.client` — the members are `private[client]`, so no other
package could construct the actor — is kept and reworded to drop the
`ClosureRequest` mention, since the rationale still holds for `ClientActor`
itself. That is the one added line in this diff. `InitializeRequest`,
`CommandRequest` and `ObservableRequest` are untouched.
### Any related issues, documentation, discussions?
Closes #7779
### How was this PR tested?
Existing tests only — this PR adds none, since it removes code and the tests
that covered it.
Locally, from the repo root with Java 17:
- `sbt "WorkflowExecutionService/Test/compile"` — success.
- `sbt "WorkflowExecutionService/testOnly *ClientActorSpec"` — 4 tests, all
pass.
- `sbt scalafmtCheckAll "scalafixAll --check"` — clean (this PR edits
surviving files, so the unused-import gate matters).
Verification, re-runnable by a reviewer:
```
git grep -n ClosureRequest # only the deleted declaration, arm, import
and tests
git grep -n 'clientActor !' # AmberClient sends only PoisonPill and
CommandRequest
```
> CI note: build jobs may fail repo-wide at workflow startup while the
injected `carabiner-dev/actions/install/ampel` action is off the ASF allowlist
— `main` fails identically. Same class as #6989 and #7572, unrelated to this
change.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude 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]
