feat(NODE-7223): run checkout on connect regardless of credentials #4715
Conversation
durran
added
the
Primary Review
test/unit/assorted/server_discovery_and_monitoring.spec.test.ts
Outdated
Show resolved
Hide resolved
durran
added
Team Review
src/mongo_client.ts
Outdated
| * An optional method to verify a typical set of preconditions before using a MongoClient. | ||
| * For detailed information about the connect process see the MongoClient.connect static method documentation. |
There was a problem hiding this comment.
suggest just copying the doc comment from MongoClient.connect(). Or reworking this a bit.
But the phrasing "to verify a typical set of preconditions" feels odd (and also - typical set of preconditions for who and what, exactly?).
Alternatively, since I expect users primarily use the instance connect and not the static connect, we could just move the docs from the static method here and just link the static method's documentation to this.
Thoughts?
There was a problem hiding this comment.
I did try to link and api-extractor throws because the syntax is abiguous, you have to use tsdoc syntax which means you need to add a new flag to typedoc to get it to link properly. Probably something worth looking into adding if linking around to other docs would be valuable.
I can move the docs to the instance or the static or copy them on to both?
updated the preconditions bit
test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts
Show resolved
Hide resolved
|
|
||
| it( | ||
| 'does not checkout connection when authentication is disabled', | ||
| 'checks out connection to confirm connectivity even when authentication is disabled', |
There was a problem hiding this comment.
can we just combine this test with the one above?
There was a problem hiding this comment.
I'm not sure what you mean by combine? because one is auth disabled and the other one is enabled, how are they combinable unless you want to move the metadata filter check into the test?
There was a problem hiding this comment.
There is no longer a difference in behavior when auth is enabled or disabled, right? So we don't need two tests. One test that runs with both auth enabled and disabled is sufficient.
There was a problem hiding this comment.
Oh right of course it'll run in both without the filter
Description
Summary of Changes
Run checkout always in connect
Notes for Reviewers
What is the motivation for this change?
Release Highlight
Improve
MongoClient.connect()consistency across environmentsThe MongoClient connect function will now run a handshake regardless of credentials being defined. The upshot of this change is that connect is more consistent at verifying some fail-fast preconditions regardless of environment. For example, previously, if connecting to a
loadBalanced=truecluster without authentication there would not have been an error until a command was attempted.Release notes highlight
Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript