-
Notifications
You must be signed in to change notification settings - Fork 51
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-9029: remove retryWrites from uri connection string #68
DOCSP-9029: remove retryWrites from uri connection string #68
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.
LGTM with one nit about useUnifiedTopology
@@ -4,7 +4,7 @@ const stream = require("stream"); | |||
|
|||
// Replace the following string with your MongoDB deployment's connection string. | |||
const uri = | |||
"mongodb+srv://<user>:<password>@<cluster-url>?retryWrites=true&w=majority&useUnifiedTopology=true"; | |||
"mongodb+srv://<user>:<password>@<cluster-url>?w=majority&useUnifiedTopology=true"; |
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.
while we're here, I think you should remove useUnifiedTopology=true
from the connection string as well. It's likely by the time these docs are released we will will have released v4 of the driver (where this won't be required anymore), or users will have already enabled the flag. Just trying to spare you the pain of having a new PR to modify every one of these strings again.
Also, FWIW the connection strings in this PR aren't consistent in terms of use of this connection string option.
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'll remove all the instances of useUnifiedTopology=true
and replace the example in fundamentals/connection with another parameter.
Thanks for noticing the variance in connection strings. I think it's ok to feature different connection string parameters across examples for now. Perhaps in the future, we'll have a better way to indicate -- whether implicitly or explicitly -- how to specify parameters in the connection string.
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.
Would you mind taking a quick look at the changes I made in response to your comment when you get the chance? @mbroadst
Connection guide reference to useUnifiedTopology is in this PR: #69 |
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.
LGTM!
* DOCSP-9029: remove retryWrites from uri connection string
JIRA: https://jira.mongodb.org/browse/DOCSP-9029
Staging: https://docs-mongodbcom-staging.corp.mongodb.com/7496942/node/docsworker/DOCSP-9029-remove-retrywrites/