-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Bugfix/2897 fix exponential backoff #2903
Conversation
@@ -41,7 +41,8 @@ public class InfoUpdateTrigger extends AbstractEventHandler<InstanceEvent> { | |||
public InfoUpdateTrigger(InfoUpdater infoUpdater, Publisher<InstanceEvent> publisher) { | |||
super(publisher, InstanceEvent.class); | |||
this.infoUpdater = infoUpdater; | |||
this.intervalCheck = new IntervalCheck("info", this::updateInfo, Duration.ofMinutes(5), Duration.ofMinutes(1)); | |||
this.intervalCheck = new IntervalCheck("info", this::updateInfo, Duration.ofMinutes(5), Duration.ofMinutes(1), | |||
Duration.ofMinutes(10)); |
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 don't like that we have defaults here (because we already have them in the AdminServerProperties).
In the AdminServerAutoConfiguration we call the setters to override these values anyways.
Can't we just pass the values to this constructor here? Then we could get rid of these defaults here and also get rid of the setters which are just passing the value to the next object.
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.
hm, the setters are also used in tests, makes them easier
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.
changed this, but kept the setters for tests
closes #2897 |
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
No description provided.