Skip to content
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

Parse duration config strings like 1s into kotlin.time.Duration #37562

Closed
dpozinen opened this issue Sep 24, 2023 · 9 comments
Closed

Parse duration config strings like 1s into kotlin.time.Duration #37562

dpozinen opened this issue Sep 24, 2023 · 9 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@dpozinen
Copy link

With an application.yml like so:

my-app:
  duration: 4h

and a class property like so:

import kotlin.time.Duration

@Service
class MyClass(
   @Value("\${my-app.duration}") private val followDuration: Duration
)

getting

Unsatisfied dependency expressed through constructor parameter 3: Failed to convert value of type 'java.lang.String' to required type 'long'; For input string: "PT4h"
...
Caused by: org.springframework.beans.TypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'long'; For input string: "PT4h"
...
Caused by: java.lang.NumberFormatException: For input string: "PT4h"

Spring boot version: 3.1.4
Kotlin: 1.9.20-Beta

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 24, 2023
@Sap004

This comment was marked as resolved.

@wilkinsona wilkinsona changed the title feature request: parse duration config strings like 1s into kotlin.time.Duration Parse duration config strings like 1s into kotlin.time.Duration Sep 25, 2023
@dpozinen

This comment was marked as resolved.

@wilkinsona wilkinsona added the type: enhancement A general enhancement label Sep 25, 2023
@mhalbritter mhalbritter removed the status: waiting-for-triage An issue we've not yet triaged label Sep 25, 2023
@mhalbritter mhalbritter added this to the 3.x milestone Sep 25, 2023
@Sap004
Copy link

Sap004 commented Sep 25, 2023

@dpozinen I am currently working on addressing this issue. I'll be submitting a pull request shortly for review

@wilkinsona
Copy link
Member

I'd like to keep this as waiting for triage until we know what's necessary. If it requires further Kotlin code we'll need to consider if the potential benefits justify that complexity. There's also the Framework side of things to consider such as spring-projects/spring-framework#30396.

@wilkinsona wilkinsona added the status: waiting-for-triage An issue we've not yet triaged label Sep 25, 2023
@wilkinsona
Copy link
Member

wilkinsona commented Sep 25, 2023

@Sap004 please be aware that, unfortunately, there's a good chance that a PR will be declined at this time. Things are moving towards duration parsing being handling in Spring Framework. spring-projects/spring-framework#30396 is tracking that and we may not want to add any more Boot-specific features in this area until that PR's been resolved one way or another.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Sep 25, 2023
@wilkinsona wilkinsona removed this from the 3.x milestone Sep 25, 2023
@philwebb philwebb changed the title Parse duration config strings like 1s into kotlin.time.Duration Parse duration config strings like 1s into kotlin.time.Duration Sep 27, 2023
@philwebb
Copy link
Member

We discussed this today as a team and decided that we don't think this is a good idea for us to implement. The fact that our current DurationStyle enum is tied to java.time. types along with the fact we have a @DurationUnit annotation that clashes with a similarly named Kotlin type makes us think that this will be confusing for users.

Since Kotlin has an extension method to convert a java.time.Duration to a kotlin.time.Duration we recommend that the Java type is used for binding and extension method be used whenever a Kotlin type is needed.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Sep 27, 2023
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Sep 27, 2023
@neeme-praks-sympower
Copy link

Since Kotlin has an extension method to convert a java.time.Duration to a kotlin.time.Duration we recommend that the Java type is used for binding and extension method be used whenever a Kotlin type is needed.

The best thing about kotlin.time.Duration is the support for human readable period definitions such as 1.5s. Compare that to PT1.5S. More readable format is especially important in case of configuration files (the main use-case for me). Couldn't you just detect that if the target type is kotlin.time.Duration then parse it as the Kotlin type?

@philwebb
Copy link
Member

philwebb commented Apr 4, 2024

@neeme-praks-sympower We're not keen to add any more complexity to the Duration parsing in Spring Boot, but I do think the period parsing is interesting. I've added a comment to spring-projects/spring-framework#30396 (comment) to see if Spring Framework can consider that feature for java.time.Duration parsing.

@simonbasle
Copy link

simonbasle commented Apr 17, 2024

I plan on getting back on the Spring Framework side of things soon, and I'll be only talking from Framework perspective here.

Regarding the Kotlin-style parsing, I fear this might be a bit too involved to fully support decimal parts. And without it, the simple style seems enough?

I also wonder if the other Kotlin style - the one where multiple components in different units can be combined, separated by a space - would be easier to parse AND more human readable. any opinion on that one ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

8 participants