SWIFT-754: Use Int in public API as much as possible #455
Conversation
| public var max: Document? | ||
|
|
||
| /// The maximum amount of time for the server to wait on new documents to satisfy a tailable cursor | ||
| /// query. This only applies when used with `CursorType.tailableAwait`. Otherwise, this option is ignored. |
There was a problem hiding this comment.
while you are here, how about updating this to "The maximum amount of time, in milliseconds, for the server..."? (same for similar time based MS options you've updated). most people probably know what MS are but I think it would be good to clarify.
There was a problem hiding this comment.
Do we have a way of logging a warning? but I agree its acceptable and pure swift is on the horizon which would make this a non-issue.
There was a problem hiding this comment.
I think you maybe posted this in reply to the wrong comment?
In any case we don't have logging right now, but you can put a TODO SWIFT-349 saying to log something when we add it
There was a problem hiding this comment.
Um yea that was meant for mongoc_apm_command_succeeded_get_duration.... not sure how that happened sounds good 👍
| switch self { | ||
| case let .number(wNumber): | ||
| try container.encode(wNumber) | ||
| try container.encode(Int32(wNumber)) |
There was a problem hiding this comment.
since this method already throws anyway, we could consider throwing an error here if the value is too large to cast to an Int32. it's unlikely anyone will ever hit the error case but it's a little safer than the cast which could technically fail.
you could use the Int32(exactly:) initializer to test this safely, which returns nil if the value can't be represented exactly as an Int32.
also, let's add a comment here clarifying we do this because libmongoc requires it. then when we aren't wrapping libmongoc anymore we will know we can stop doing the cast.
|
|
||
| fileprivate init(mongocEvent: MongocCommandSucceededEvent) { | ||
| self.duration = mongoc_apm_command_succeeded_get_duration(mongocEvent.ptr) | ||
| self.duration = Int(mongoc_apm_command_succeeded_get_duration(mongocEvent.ptr)) |
There was a problem hiding this comment.
I guess technically this can fail if we're on a 32 bit system and a command lasts longer than 35 minutes... probably not gonna happen though
| internal init(_ description: OpaquePointer) { | ||
| self.address = Address(mongoc_server_description_host(description)) | ||
| self.roundTripTime = mongoc_server_description_round_trip_time(description) | ||
| self.roundTripTime = Int(mongoc_server_description_round_trip_time(description)) |
There was a problem hiding this comment.
similar to the command duration one this seems ok too... this one is even less likely to be an issue because it's milliseconds rather than microseconds
| try container.encode(wNumber) | ||
| } else { | ||
| throw InvalidArgumentError( | ||
| message: "Invalid WriteConcern.w \(wNumber): must be between \(Int32.min) and \(Int32.max)" |
There was a problem hiding this comment.
the number actually has to be >= 0
https://jira.mongodb.org/browse/SWIFT-754
A more swifty driver with
Intdropping the bit width specification in the user API.