-
Notifications
You must be signed in to change notification settings - Fork 266
PHPLIB-438: Unify handling for write stages in aggregation pipelines #644
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
Conversation
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.
Some documentation changes are needed here (both RST and maybe Aggregate
's doc blocks). We have quite a few references to $out
where we reference it as the only write operator.
Based on https://docs.mongodb.com/master/reference/method/db.collection.aggregate/, the manual started referring to "$out
and $merge
stages", so we can probably do the same.
Review items applied, documentation updated. I also added read and write concerns to the |
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.
Question about Context::fromCrud()
, but feel free to leave as-is if it's truly needed. Folks in #driver-devs
may be able to confirm whether those options should be necessary.
|
||
$o->defaultWriteOptions = [ | ||
'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), | ||
]; |
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.
Was this (and outcomeFindOptions
) actually needed? AFAIK, all of the CRUD tests read/write to the primary and never specified a custom read concern or read preference.
AFAIK, defaultWriteOptions
was only used for transaction tests (per implementation instructions).
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 added this after a test kept failing on a replicaset setup. I got suspicious after I couldn’t replicate the failures when step-debugging through the test and thought that the problem might be with the secondaries being read before they have the data.
https://jira.mongodb.org/browse/PHPLIB-438