Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
DOCSP-47835 Clarify batchsize behavior #215
DOCSP-47835 Clarify batchsize behavior #215
Changes from 10 commits
4d29070
bf79b9e
b02a39a
248115f
6f46523
a7bd71c
269819f
76d9bd5
69d3ed4
c18ef44
e9aab32
6183d86
89b304c
635e787
debcfe1
380a16e
892f7ea
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: should you also remove this sentence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call out, I'll remove!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice this earlier, but this paragraph seems like a relevant bit of information to preserve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this section is talking about both
find()
andfindOne()
, I think this can just refer to "thefind
command" (used by both helpers).Therefore, this snippet will just duplicate the first paragraph from the
batchSize
docs inMongoDBCollection-find.txt
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The special behavior of
batchSize: 0
is notable, and also discussed in thefind
command docs. Particularly withaggregate
operations, there is a legitimate use case for wanting to start the aggregation on the server without waiting for the first result to become available. Allowing a cursor to be created immediately and iterated later (viabatchSize: 0
satisfies that).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should definitely specify:
As-is, it's inconsistent with the other docs snippets and could mislead users into thinking this has no effect on
getMore
for subsequent batches.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I: remove
aggregate
from this description as its about the find method. I think JM meant that setting this option is useful for aggregation contexts but that users might use it for finds tooThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can discuss
batchSize: 0
here as we do for thefind()
option, since there is a legitimate use case for doing so. I'm OK with keeping this in the API reference and out of the prose docs (e.g.read/retrieve.txt
), since it's more technical.There is also a special directive in the CRUD spec for drivers to intentionally omit
batchSize
for pipelines that include$out
or$merge
, since it might prevent the pipeline from executing (and writing output). I don't think we need to mention that, though, as it is enforced internally (see: Aggregate.php).