pjfanning commented on PR #1320:
URL: https://github.com/apache/pekko-http/pull/1320#issuecomment-5888138774
Thanks for the quick update @Rayan-and-beyond, having a proper public
setting with a Java API is much better.
One suggestion on the Java duration conversions. The new getter/setter
round-trip through milliseconds (copying the existing
`getCompletionTimeout`/`withCompletionTimeout` lines):
```scala
def getPersistentConnectionMaxAge: Duration =
Duration.ofMillis(persistentConnectionMaxAge.toMillis)
def withPersistentConnectionMaxAge(maxAge: Duration): Http2ClientSettings =
self.withPersistentConnectionMaxAge(maxAge.toMillis.millis)
```
This truncates sub-millisecond precision. Since `0` means "disabled" for
this setting, a tiny positive age passed from Java would silently disable the
feature. `java.time.Duration.toMillis` also throws an `ArithmeticException` for
very large values.
`scala.jdk.DurationConverters` (standard library in Scala 2.13 and 3,
already used in http-core, e.g. `ConnectionPoolSettings`,
`ClientConnectionSettings`, `ServerBinding`) converts losslessly in both
directions:
```scala
import scala.jdk.DurationConverters._
def getPersistentConnectionMaxAge: java.time.Duration =
persistentConnectionMaxAge.toJava
def withPersistentConnectionMaxAge(maxAge: java.time.Duration):
Http2ClientSettings =
self.withPersistentConnectionMaxAge(maxAge.toScala)
```
If the setting ever needs to support `infinite`,
`pekko.http.impl.util.JavaDurationConverter` should be used instead: its
`toJava` maps `Duration.Inf` to `ChronoUnit.FOREVER.getDuration`, and #1316
adds the inverse `toScala`. On that note, #1316 uses `infinite` to disable the
server-side `max-connection-age`, while this PR uses `0s`. It may be worth
aligning the two once #1316 is merged.
The existing `completionTimeout` conversions have the same issue but can be
left as they are in this PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
