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: Change socket timeout default to 0 #2564

Merged
merged 1 commit into from Oct 9, 2020

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Oct 5, 2020

Socket timeout by default should be infinity.
Removed 6min defaults.

NODE-2835

Socket timeout by default should be infinity.

NODE-2835
@nbbeeken nbbeeken requested review from reggi and emadum October 5, 2020 17:55
Copy link
Contributor

@reggi reggi left a comment

Choose a reason for hiding this comment

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

LGTM

Maybe worth adding a @default tag to the Client options interface.

https://github.com/mongodb/node-mongodb-native/blob/master/src/mongo_client.ts#L78

@nbbeeken
Copy link
Contributor Author

nbbeeken commented Oct 7, 2020

LGTM

Maybe worth adding a @default tag to the Client options interface.

https://github.com/mongodb/node-mongodb-native/blob/master/src/mongo_client.ts#L78

I would but as of now TSDoc doesn't have default 😅 support for this tag microsoft/tsdoc#27, we could always enable it as a custom tag but we need a pipeline that does something with the information.

I can use the @default tag when I port this to 3.6 however! I'll be sure to remember that.

Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

👍

@nbbeeken nbbeeken merged commit 7ed6dbf into master Oct 9, 2020
@nbbeeken nbbeeken deleted the NODE-2835/remove-default-read-timeout branch October 9, 2020 15:52
@mbroadst
Copy link
Member

@nbbeeken I would have expected some tests here, can you please add one/some?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants