-
Notifications
You must be signed in to change notification settings - Fork 4
Upgrade to MongoDB Driver v6 #101
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
|
has this been tested for interoperability with the multiple backend libs that use |
|
@aljones15 the types of changes you mentioned related to driver API return value changes etc will still need to be addressed in relying libs. |
ok well maybe those existing PRs can be revised to work with driver 6. thanks for this PR. |
dlongley
left a comment
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.
Thanks!
| serverSelectionTimeoutMS: 30000, | ||
| autoReconnect: false, | ||
| useNewUrlParser: true, | ||
| // promotes binary BSON values to native Node.js buffers |
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.
Did we mean to get rid of this comment? It's useful, IMO.
| await Promise.all(unopened.map(async name => { | ||
| // Note: We only pass `{writeConcern}` here to get around a bug in mongodb | ||
| // node driver 3.6.4 where `writeConcern` from `db` is not passed to | ||
| // collection |
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.
Presumably this bug has been fixed now.
|
Some of the changes in this PR for v6 need to be adjusted ( |
dlongley
left a comment
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.
See #106 and latest comment here.
|
Closing in favor of #107, which includes some of the non-conflicting commits from this PR. |
Thanks to @aljones15 for previous work in exploring the driver upgrade.
This implementation:
insertAPIs no longer return the document that was inserted.This has been tested with one top level app. I will be plugging it into additional Bedrock libs before release.