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

Duration, SizeUnit could implement Serializable #2971

Closed
cartermckinnon opened this issue Oct 10, 2019 · 3 comments · Fixed by #2975
Closed

Duration, SizeUnit could implement Serializable #2971

cartermckinnon opened this issue Oct 10, 2019 · 3 comments · Fixed by #2975
Assignees
Milestone

Comments

@cartermckinnon
Copy link

I often use Duration and SizeUnit for configuration parameters. Because these classes do not implement java.io.Serializable, they cause issues when Jackson isn't the serializer. For example, they cause exceptions in Spark jobs when utilized as part of the task (i.e. in a lambda).

I believe that implementing Serializable on these classes is low impact, since the interface is purely semantic. Adding a serialVersionUID would be the only other change. Happy to PR this if someone agrees?

@mattnelson
Copy link
Contributor

For Duration I would recommend using the JDK instead, which is already serializable. In fact it may be worth the discussion to see if dropwizard's Duration should be deprecated or modified to extend the JDK Duration.

https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html

@cartermckinnon
Copy link
Author

cartermckinnon commented Oct 11, 2019

My use case is solely configuration; I prefer something friendly like 5 minutes (in YAML, an environment variable, etc.) instead of the ISO-8601 duration format (PnDTnHnMn.nS). I agree that the JDK option should be chosen when possible, but I think the two implementations have different goals. There are various work arounds for the limitation I've hit, so maybe it's not worth addressing upstream at this point. 🤷‍♂

joschi added a commit that referenced this issue Oct 12, 2019
@joschi joschi added this to the 2.0.0 milestone Oct 12, 2019
@joschi joschi self-assigned this Oct 12, 2019
@cartermckinnon
Copy link
Author

Thank you, @joschi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants