[JAVA-3897] Wrong type for ClusterSettings.Builder#connectTimeOut and readTimeout parameter Created: 25/Nov/20 Updated: 19/Jan/24 Resolved: 04/Jan/24 |
|
| Status: | Closed |
| Project: | Java Driver |
| Component/s: | Builders |
| Affects Version/s: | 4.1.1 |
| Fix Version/s: | 5.0.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Óscar Burgos | Assignee: | Valentin Kavalenka |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Epic Link: | 5.0 Breaking Changes | ||||||||||||
| Quarter: | FY24Q4 | ||||||||||||
| Backwards Compatibility: | Major Change | ||||||||||||
| Documentation Changes: | Needed | ||||||||||||
| Documentation Changes Summary: | Let's mention in What's new the following API changes that break binary compatibility (the existing compiled code that call these methods has to be recompiled; no source code changes are needed): `com.mongodb.connection.SocketSettings.Builder.connectTimeout`/`readTimeout` now have the first parameter of the `long` type instead of `int`. |
||||||||||||
| Description |
|
I think that timeouts parameters of methods on SocketSettings.Builder.Builder should be long,, not int, as it happens on ConnectionPoolSettings.Builder or Clustersettings.Builder
For example, SocketSettings.Builder#connectTimeout(int, java.util.concurrent.TimeUnit) and SocketSettings.Builder#readTimeout(int, java.util.concurrent.TimeUnit) sulf by SocketSettings.Builder#connectTimeout(long, java.util.concurrent.TimeUnit) and SocketSettings.Builder#readTimeout(long, java.util.concurrent.TimeUnit) |
| Comments |
| Comment by Githook User [ 04/Jan/24 ] | ||||||||||||||||
|
Author: {'name': 'Valentin Kovalenko', 'email': 'valentin.kovalenko@mongodb.com', 'username': 'stIncMale'}Message: Accept `long` instead of `int` in `SocketSettings.Builder.connectTimeout`/`readTimeout` (#1279)
| ||||||||||||||||
| Comment by Jeffrey Yemin [ 14/Aug/23 ] | ||||||||||||||||
|
We can also consider using java.time.Duration here. | ||||||||||||||||
| Comment by Jeffrey Yemin [ 25/Nov/20 ] | ||||||||||||||||
|
I'll put this on the list for the 5.0 release, when we can next make breaking changes. Just be advised that we only recently recently 4.0, so 5.0 will not be coming soon. | ||||||||||||||||
| Comment by Jeffrey Yemin [ 25/Nov/20 ] | ||||||||||||||||
|
Yeah, I'm sorry for that. You're going to have to cast to an int in this case. In practice it should be safe as the max integer value comes out to ~596 hours, which is far longer than any timeout anyone will ever need. | ||||||||||||||||
| Comment by Óscar Burgos [ 25/Nov/20 ] | ||||||||||||||||
|
My use case is set the value from Duration more easily, and also for consistency. For example, I can do this:
but I can't do this other:
| ||||||||||||||||
| Comment by Jeffrey Yemin [ 25/Nov/20 ] | ||||||||||||||||
|
oscar.burgos-martin@capgemini.com Thanks for opening this. Unfortunately I don't think we can change it now as it would break binary compatibility to do so. Do you have an actual use case where you need it to be a long or is it just about consistency? |