Accepting java.time.Duration as alternative to milliseconds in arguments

[aahlenst]

It would be nice if all kinds of intervals and durations could be expressed as java.time.Duration in addition to a long of milliseconds. It would make it easier to understand code using Spring Retry when reading it because Duration.ofMinutes(5) is more obvious than 300_000 or 5*60*1000. Furthermore, the JDK has already adopted java.time.Duration for timeouts, for example, in Thread.sleep(Duration).

To clarify what I have in mind, an example involving RetryTemplateBuilder#fixedBackoff(long):

public RetryTemplateBuilder fixedBackoff(Duration interval) {
    Objects.requireNonNull(interval, "interval is null");
    return this.fixedBackoff(interval.toMillis());
}

Because it's a rather broad change, I'd like to know whether that's something you would accept and whether you rather want smaller steps (like changing RetryTemplateBuilder first, policies later) or everything in one PR.

[artembilan]

This is rather too big and much involved. More over it is going to be some kind of breaking change. We cannot have setters with the same name and different types: the XML parser does not understand which one to chose. There is also no built-in support in Spring to parse duration strings automatically.

So, even if we accept this as feature improvement, it is not going to happen in the current version. I would say that probably somewhere in 3.0.

If I would do this change, I would start from the bottom. For example, TimeoutRetryPolicy, then Sleeper and BackOff.

[snicoll]

There is also no built-in support in Spring to parse duration strings automatically.

That's going to change in 6.1 hopefully but we need to have that as a base indeed before considering that change.

Can we maybe update the code that's not used by XML parsers for a start?

[artembilan]

We cannot guess that because even simple <bean> for the mentioned TimeoutRetryPolicy would fail if we had setters like setTimeout(long) and setTimeout(Duration). It would be a bit awkward to have a new one like setTimeoutDuration(). Replacing long with Duration will be a breaking change and that's why I call it 3.0 theme. The mention builder in the beginning could probably introduce a Duration variants because its usage in the XML configuration is really hard. But will it justify that change since we won't be able to apply such a Duration feature to target objects like UniformRandomBackOffPolicy or TimeoutRetryPolicy.

The @Backoff also have long attributes where for duration support we would need to change them to strings.

[snicoll]

Can we maybe update the code that's not used by XML parsers for a start?

I meant code that's used programmatically (without setter like injection). Maybe we don't have a lot of those.

[aahlenst]

Replacing long with Duration will be a breaking change and that's why I call it 3.0 theme.

I'm not sure if it is worth the effort and the upgrading pain because everyone has to change their code. I envisioned some overloads because that would save me a few keystrokes (Duration.ofMinutes(5).toMillis()) without breaking anyone's code or polluting the API (like setTimeoutDuration() would). But I wasn't aware that the XML configuration cannot distinguish overloads.

[artembilan]

Yeah... So, let's start then from the RetryTemplateBuilder introducing new methods based on a Duration even if internally we still have to convert it back to the toMillis() to satisfy the other API which we cannot change at the moment.

[aahlenst]

PR is up: #344

[quaff]

There is also no built-in support in Spring to parse duration strings automatically.

FYI, It's introduced in 6.2, see https://github.com/spring-projects/spring-framework/pull/30396

[artembilan]

Thank you for the pointer, @quaff . Unfortunately we cannot upgrade there yet. For that we would need a new version to start for this project. We have some plans on the matter in the future anyway.