aglinxinyuan opened a new pull request, #8332:
URL: https://github.com/apache/texera/pull/8332

   ### What changes were proposed in this PR?
   
   Deletes two of the three `RecoveryPayload` subtypes — `UpdateRecoveryStatus` 
and `ResendOutputTo` — which have no sender and no handler. Pure deletion, no 
behaviour change: **−39 lines**.
   
   `NotifyFailedNode` is the only subtype still in use (sent from `AmberClient` 
when a cluster node fails), and it stays, as does the sealed trait.
   
   **Provenance.** Both arrived with #1677 (2022-11-07, "Amber Fault Tolerance: 
Global Recovery and Detection") and were genuinely used — the coordinator 
matched on them with `case UpdateRecoveryStatus(isRecovering)` and `case 
ResendOutputTo(vid, ref)`. #2208 (2023-11-15, "Refactoring of amber engine") 
removed those handler arms along with the old recovery path; from that commit 
on, the only file mentioning either name is `RecoveryPayload.scala` itself. 
Dead for about three years. They picked up unit tests in #4829 (2026-05-03) 
during the coverage work, which is why they look live.
   
   Removing them also frees the `ActorRef` and `ActorVirtualIdentity` imports, 
which only `ResendOutputTo` used.
   
   > Reviewer note — the spec is edited, not just trimmed. 
`AmberMessageEnvelopesSpec` used `UpdateRecoveryStatus` as the payload fixture 
in its "WorkflowRecoveryMessage carries sender and payload" test; that now uses 
`NotifyFailedNode`, so the envelope contract stays covered. The suite-local 
`ActorSystem` and its `afterAll` existed **only** to give the `ResendOutputTo` 
test a real `ActorRef`, so they go with it — which also drops the 
`BeforeAndAfterAll` mixin and the pekko `ActorSystem`/`TestKit` imports. The 
remaining 10 tests pass.
   
   ### Any related issues, documentation, discussions?
   
   Closes #8329
   
   ### How was this PR tested?
   
   Existing tests only — this PR adds none; it removes two types and rewrites 
the assertions that referenced them.
   
   Locally, from the repo root with Java 17:
   
   - `sbt "WorkflowExecutionService/Test/compile"` — success.
   - `sbt "WorkflowExecutionService/testOnly *AmberMessageEnvelopesSpec"` — 10 
tests, all pass.
   - `sbt scalafmtCheckAll "scalafixAll --check"` — clean.
   
   Verification, re-runnable by a reviewer:
   
   ```
   git grep -n "UpdateRecoveryStatus\|ResendOutputTo"   # only the deleted 
declarations and their assertions
   git grep -n "NotifyFailedNode" -- amber/src/main     # the live subtype, 
untouched
   ```
   
   ### 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]

Reply via email to