feat: Added GetBulkAsRawMap function to the MongoDB API #259
Conversation
GetBulkAsRawMap returns a slice of 'raw' maps, one for each of the specified keys. Also exposed PrepareFilter and CreateMongoDBFindOptions so that clients may prepare queries for use with the QueryCustom function. Signed-off-by: Bob Stasyszyn <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #259 +/- ##
==========================================
+ Coverage 85.82% 88.23% +2.41%
==========================================
Files 26 1 -25
Lines 3992 799 -3193
==========================================
- Hits 3426 705 -2721
+ Misses 334 52 -282
+ Partials 232 42 -190 Continue to review full report at Codecov.
|
| ctxWithTimeout, cancel := context.WithTimeout(context.Background(), s.timeout) | ||
| defer cancel() | ||
|
|
||
| for cursor.Next(ctxWithTimeout) { |
There was a problem hiding this comment.
Something to note here: since this context is getting reused, all the cursor.Next calls need to complete before the timeout. Might be an issue if there's a large result set.
I see that I also did the same thing in the collectBulkGetResults method. Perhaps it's not urgent right now, but might be worth reworking in the future to use a new context for each call.
There was a problem hiding this comment.
Something to note here: since this context is getting reused, all the cursor.Next calls need to complete before the timeout. Might be an issue if there's a large result set.
I see that I also did the same thing in the
collectBulkGetResultsmethod. Perhaps it's not urgent right now, but might be worth reworking in the future to use a new context for each call.
It depends on how timeout is defined. If timeout is defined as the timeout for the entire function, i.e. GetBulk, then this is correct. Otherwise, you'd have a variable timeout for GetBulk depending on how many items in the result set.
There was a problem hiding this comment.
I probably need to define the timeout more explicitly. Right now here's how I defined it above the WithTimeout function:
// WithTimeout is an option for specifying the timeout for all calls to MongoDB.
// The timeout is 10 seconds by default.
When I created that option, I was thinking the timeout was per remote call to MongoDB (which a cursor.Next would be, depending on the current page). So far in the other areas of the code, I create a new timeout for every possible MongoDB call (with the exception of the bulk results collection as mentioned above). But I now see a problem with that approach. Some calls to MongoDB can take longer than others (e.g. some Find calls that execute queries). I think ideally the caller should be able to pass in their own timeout to each call, which is tricky with the current interface. I think I should create a followup issue for me to address this in the future. What do you think?
There was a problem hiding this comment.
Yes, ideally the caller should be providing the context.
GetBulkAsRawMap returns a slice of 'raw' maps, one for each of the specified keys.
Also exposed PrepareFilter and CreateMongoDBFindOptions so that clients may prepare queries for use with the QueryCustom function.
Signed-off-by: Bob Stasyszyn [email protected]