Skip to content
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

fix: restore auto direct connection behavior #2719

Merged
merged 4 commits into from
Feb 2, 2021

Conversation

emadum
Copy link
Contributor

@emadum emadum commented Jan 27, 2021

Description

We removed automatic direct connection for the unified topology in the 3.6.3 release of the driver. This change was preparatory for the 4.0 version of the driver, where we'll always perform automatic discovery. This PR restores automatic direct connection when connecting to a single host without a specified replicaSet and without directConnection: false.

In #2557 this was justified because useUnifiedTopology: true is an opt-in feature, but we've had a number of users file tickets about this change, and it's probably more appropriate to delay it until the major release of the driver, along with a call-out/explanation in the migration guide, rather than putting it in a patch release. That said, I'm open to closing this PR if we decide the original reasoning was appropriate.

NODE-2966

What changed?

Are there any files to ignore?

@emadum emadum force-pushed the NODE-2966/3.6/restore-direct-connection branch from 41677c4 to 07a3117 Compare January 27, 2021 13:49
@emadum emadum marked this pull request as ready for review January 27, 2021 13:59
Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emadum What do you think about adding a test for the specific scenario? Even with the functionality going away I think it would be beneficial not to have a regression later in the 3.6 branch. I was expecting a test where directConnection and replSet are both not supplied when connecting directly to a replica set node.

@emadum
Copy link
Contributor Author

emadum commented Jan 28, 2021

@durran Adding a test definitely seems reasonable! I'll do so once it's confirmed we all agree about reverting this change.

@nbbeeken
Copy link
Contributor

nbbeeken commented Jan 28, 2021

I agree with the change, should we consider a deprecation warning of sorts? as long as it can be hushed I think its a good place to notify users.

@emadum
Copy link
Contributor Author

emadum commented Jan 28, 2021

I agree with the change, should we consider a deprecation warning of sorts? as long as it can be hushed I think its a good place to notify users.

Just to have a record of our conversation, we aren't going to emit a deprecation warning because it would also be emitted whenever connecting to a standalone MongoDB server. However we should definitely make sure to explain this change in the 4.0 migration guide.

@emadum emadum requested a review from durran January 28, 2021 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants