Updated BackoffSupervisor to Akka JVM 2.5.18 #3678
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ismaelhamed
commented
Dec 17, 2018
| { | ||
| var supervisor = Create(OnStopOptions(maxNrOfRetries: 2)); | ||
|
|
||
| IActorRef WaitForChild(int n = 0) |
There was a problem hiding this comment.
Do we have anything like scalatest.concurrent.Eventually? That would simplify this piece of code.
45b58a0 to
31e18cb
Compare
|
@ismaelhamed is this still a work in progress? or is it ready for review? |
31e18cb to
123572a
Compare
|
@Aaronontheweb It's ready for review. I've just updated the tests with the |
123572a to
037478f
Compare
037478f to
97c2002
Compare
|
@Aaronontheweb I have another PR ready, that builds on top of this one, with #25933. This should put the BackoffSupervisor on a par with the JVM. |
|
@ismaelhamed got it - I'll review this again rq |
Aaronontheweb
approved these changes
Jan 10, 2019
| @@ -3754,22 +3755,29 @@ namespace Akka.Pattern | |||
| { | |||
| public class static Backoff | |||
| { | |||
| [System.ObsoleteAttribute("Use the overloaded one which accepts maxNrOfRetries instead.")] | |||
| }); | ||
|
|
||
| supervisor.Tell("boom"); //this will be sent to deadLetters | ||
| ExpectNoMsg(500); |
| @@ -23,9 +23,25 @@ public static class Backoff | |||
| /// <param name="minBackoff">Minimum (initial) duration until the child actor will started again, if it is terminated</param> | |||
| /// <param name="maxBackoff">The exponential back-off is capped to this duration</param> | |||
| /// <param name="randomFactor">After calculation of the exponential back-off an additional random delay based on this factor is added, e.g. `0.2` adds up to `20%` delay. In order to skip this additional delay pass in `0`.</param> | |||
| [Obsolete("Use the overloaded one which accepts maxNrOfRetries instead.")] | |||
| public abstract Akka.Pattern.BackoffOptions WithSupervisorStrategy(Akka.Actor.OneForOneStrategy supervisorStrategy); | ||
| } | ||
| public sealed class BackoffSupervisor : Akka.Pattern.BackoffSupervisorBase | ||
| { | ||
| public BackoffSupervisor(Akka.Actor.Props childProps, string childName, System.TimeSpan minBackoff, System.TimeSpan maxBackoff, double randomFactor) { } | ||
| public BackoffSupervisor(Akka.Actor.Props childProps, string childName, System.TimeSpan minBackoff, System.TimeSpan maxBackoff, Akka.Pattern.IBackoffReset reset, double randomFactor, Akka.Actor.SupervisorStrategy strategy) { } | ||
| public BackoffSupervisor(Akka.Actor.Props childProps, string childName, System.TimeSpan minBackoff, System.TimeSpan maxBackoff, Akka.Pattern.IBackoffReset reset, double randomFactor, Akka.Actor.SupervisorStrategy strategy, object replyWhileStopped = null) { } |
There was a problem hiding this comment.
So this looked like a public breaking API change to me, but upon further review - this is actually calling an internal constructor belonging to the base class and can't be directly exposed to end users.
There was a problem hiding this comment.
@Aaronontheweb So it is considered a breaking change even when just adding optional parameters to a method signature?
|
@ismaelhamed done! |
|
Correct
…Sent from my iPhone
On Jan 10, 2019, at 11:21 AM, Ismael Hamed ***@***.***> wrote:
@ismaelhamed commented on this pull request.
In src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt:
> public abstract Akka.Pattern.BackoffOptions WithSupervisorStrategy(Akka.Actor.OneForOneStrategy supervisorStrategy);
}
public sealed class BackoffSupervisor : Akka.Pattern.BackoffSupervisorBase
{
public BackoffSupervisor(Akka.Actor.Props childProps, string childName, System.TimeSpan minBackoff, System.TimeSpan maxBackoff, double randomFactor) { }
- public BackoffSupervisor(Akka.Actor.Props childProps, string childName, System.TimeSpan minBackoff, System.TimeSpan maxBackoff, Akka.Pattern.IBackoffReset reset, double randomFactor, Akka.Actor.SupervisorStrategy strategy) { }
+ public BackoffSupervisor(Akka.Actor.Props childProps, string childName, System.TimeSpan minBackoff, System.TimeSpan maxBackoff, Akka.Pattern.IBackoffReset reset, double randomFactor, Akka.Actor.SupervisorStrategy strategy, object replyWhileStopped = null) { }
@Aaronontheweb So it is considered a breaking change even when just adding optional parameters to a method signature?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
This was referenced Jan 25, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updated BackoffSupervisor to Akka JVM 2.5.18