-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(NODE-5754): allow auto select family options #4185
Conversation
a55afce
to
1f17842
Compare
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.
Changes LGTM! Can you fill out the release highlights? this is def a feature to show off 🎉
Ah yeah missed that. Updated now. |
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.
Question: should we also be using autoSelectFamily
when establishing sockets for KMS requests in the state machine?
Good question. I think the answer to that should be yes? If we agree I can add to this PR. |
@durran We could potentially consume want to spend a bit of time looking at that approach? |
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 am just putting back my request changes state since you're working on the above, given the reactions I think we're all on board with expanding this to KMS.
Using makeSocket
sounds great but you may run into more refactoring than is worth, I'm thinking about how connecting will be inside that function now and you may have to catch and re-throw in "interesting" ways, additionally, I am unsure if the socks5 behavior is precisely equivalent.
If none of that ends up being a concern GREAT! But if it is, as long as we can get the autoFamily option through we can rest this work there, and file a follow up to consolidate 🚀 Good catch @baileympearson
The refactor to use |
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.
couple of Qs
Description
Allows users to specify auto select family options passed to the underlying connections.
What is changing?
autoSelectFamily
option to the client which defaults totrue
.autoSelectFamilyAttemptTimeout
option to the client with no default.Is there new documentation needed for these changes?
Add note in documentation about the options.
What is the motivation for this change?
NODE-5754
Release Highlight
Driver now supports auto selecting between IPv4 and IPv6 connections
For users on Node versions that support the
autoSelectFamily
andautoSelectFamilyAttemptTimeout
options (Node 18.13+), they can now be provided to theMongoClient
and will be passed through to socket creation.autoSelectFamily
will default totrue
withautoSelectFamilyAttemptTimeout
by default not defined. Example:Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript