New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify backoff supervision API #26156 #26135
Conversation
Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK TO TEST' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged. For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask! |
OK TO TEST |
Test FAILed. |
Test FAILed. |
Test PASSed. |
Test FAILed. |
Test PASSed. |
@patriknw I tried to simplify the API (first commit) and consolidate the implementation a little (second commit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, just some questions and suggestions.
context.parent ! msg | ||
|
||
case msg ⇒ | ||
handleMessageToChild(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we be missing cases where
case ResetRestartCount(current) ⇒ if (current == restartCount)
vs
case ResetRestartCount(current) if current == restartCount ⇒
should fall through to case msg
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite understanding the question? I didn't change anything in this part of the implementation. I think this check is just to avoid reseting the counter when the actor has been restarted again meanwhile.
akka-actor/src/main/scala/akka/pattern/BackoffOnStopSupervisor.scala
Outdated
Show resolved
Hide resolved
akka-actor/src/main/scala/akka/pattern/BackoffOnStopSupervisor.scala
Outdated
Show resolved
Hide resolved
Test FAILed. |
Test FAILed. |
Test PASSed. |
@johanandren Last commit c4ea439 moves impls to internal package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @nvollmar
Test PASSed. |
1 similar comment
Test PASSed. |
Fixes #26156 |
A proposal to make the backoff api more selfdocumenting and hiding implementation