Skip to content
This repository was archived by the owner on May 31, 2022. It is now read-only.

Conversation

@Anemy
Copy link
Member

@Anemy Anemy commented Dec 14, 2020

COMPASS-4534 and fixes https://github.com/mongodb-js/compass/issues/2075

Node driver 3.6.3 introduced a change where it no longer sets directConnection on the parsed connection string for single host connections with no supplied replica set:
mongodb/node-mongodb-native@f8fd310

This PR introduces that behavior into our connect method so existing Compass connections are not broken.

Since tests in this module aren't very integrated with the driver (yet). We have some manual tests to run:
Before merging:

  • Tested srv connection
  • Tested connecting to a standalone
  • Tested connecting directly to primary in replica set with host resolving
  • Tested connecting directly to primary in replica set without host resolving
  • Tested connecting directly to primary with ssh in replica set with host resolving
  • Tested connecting directly to primary with ssh in replica set without host resolving
  • Tested connecting directly to secondary in replica set with host resolving
  • Tested connecting directly to secondary in replica set without host resolving - this fails now even with readPreference=secondary - I think it's another change we need to make
  • Tested connecting directly to secondary with ssh in replica set with host resolving
  • Tested connecting directly to secondary with ssh in replica set without host resolving
    We'll include an integration test for this behavior in the upcoming connectivity integration test epic.

"async": "^3.1.0",
"debug": "^4.1.1",
"lodash": "^4.17.15",
"mongodb": "^3.6.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is why you were asking if we should make that a peer dep right? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. I can update that in another PR though to keep things simple. (Maybe it'll require a major release?)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v1.24.1] Can not connect DB

5 participants