RCBC-441: interpret Integer as milliseconds in duration context #110
Conversation
For example, all timeout arguments are durations in milliseconds
|
Can one of the admins verify this patch? |
| if number_or_duration.respond_to?(:in_milliseconds) | ||
| number_or_duration.public_send(:in_milliseconds) | ||
| else | ||
| number_or_duration | ||
| end.to_i |
There was a problem hiding this comment.
This breaks all the things, unfortunately. I've monkeypatched this method in our local setup and the result of converting everything using .to_i means that nil values never make it out. This method will always return an Integer. So if the client specifies nothing at all for the timeout the default value for the timeout will not be used.
Locally, it means that every attempt to connect to the cluster results in this:
Failure/Error: cluster = Cluster.connect("couchbase://#{host}?enable_dns_srv=false", couch_options)
Couchbase::Error::UnambiguousTimeout:
unable open cluster at 127.0.0.1: unambiguous_timeout (14)
I've changed this locally as follows:
result = if number_or_duration.respond_to?(:in_milliseconds)
number_or_duration.public_send(:in_milliseconds)
else
number_or_duration
end
result.nil? ? result : result.to_iThere was a problem hiding this comment.
Tagging @avsej and @DemetrisChr just to make sure this comment wasn't missed. Seems like a potentially serious issue.
There was a problem hiding this comment.
hi @mikesnare, sorry I didn't notice comment in closed PR. I've fixed nil condition in this PR: #111
For example, all timeout arguments are durations in milliseconds