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

Introduce DurationEditor for java.time.Duration support #28084

Closed

Conversation

dfa1
Copy link

@dfa1 dfa1 commented Feb 20, 2022

Recently, in a project that is using spring-framework (no spring-boot), I saw a @Configuration class with
a lot of configuration about timeouts injected as plain long (most likely milliseconds but it is not explicitly documented, as often it happens):

@Value("${connect.timeout}")
long connectTimeout;

With this small patch, it would be possible to directly inject a Duration:

@Value("${connect.timeout}")
Duration connectTimeout;

In my opinion, this is less error prone, as usually these configuration values must be adjusted with some calculations (i.e. seconds to milliseconds).

This patch allows two formats:

  • ISO 8601 format with PT prefix, e.g. PT10s, PT1m30s (as Duration.parse);
  • simplified format without PT prefix, e.g. 10s, 1m, 2h (IMHO this format is very readable for properties files).

Comments and feedback is welcome. I'm not sure about using a property editor, perhaps a Converter is a better choice?
And I'm not sure if this should be mentioned somewhere in the spring-reference... please advice :-)

@pivotal-cla
Copy link

@dfa1 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 20, 2022
@pivotal-cla
Copy link

@dfa1 Thank you for signing the Contributor License Agreement!

1 similar comment
@pivotal-cla
Copy link

@dfa1 Thank you for signing the Contributor License Agreement!

@dfa1 dfa1 force-pushed the proposal-duration-support-in-value branch from e7a028a to 558269c Compare February 20, 2022 20:38
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 25, 2022
@dfa1 dfa1 force-pushed the proposal-duration-support-in-value branch from e802f0d to e4cdef2 Compare May 4, 2022 18:34
@dfa1
Copy link
Author

dfa1 commented May 4, 2022

@rstoyanchev just reworked a bit the commit, can you please have a look?

@dfa1 dfa1 force-pushed the proposal-duration-support-in-value branch from e4cdef2 to ee75518 Compare May 4, 2022 18:46
@dfa1 dfa1 force-pushed the proposal-duration-support-in-value branch from ee75518 to 8ddb228 Compare May 4, 2022 18:54
@sbrannen sbrannen changed the title Support for java.time.Duration for @Value Introduce DurationEditor for java.time.Duration support May 5, 2022
@snicoll
Copy link
Member

snicoll commented Aug 27, 2023

@dfa1 thaks for the PR and sorry for the delay. We've been working on an extensive review of duration support in #30396. I unfortunately am going to close this PR in favor of ours.

@snicoll snicoll closed this Aug 27, 2023
@snicoll snicoll 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 or decided on labels Aug 27, 2023
@dfa1 dfa1 deleted the proposal-duration-support-in-value branch August 27, 2023 16:42
@dfa1
Copy link
Author

dfa1 commented Aug 27, 2023

@snicoll no worries... the new PR is much better and complete! 😇

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants