-
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
remove clean connectionCheckout timeout #4274
base: NODE-6090
Are you sure you want to change the base?
Conversation
Co-authored-by: Warren James <[email protected]>
Co-authored-by: Bailey Pearson <[email protected]>
Co-authored-by: Neal Beeken <[email protected]> Co-authored-by: Bailey Pearson <[email protected]>
Co-authored-by: Neal Beeken <[email protected]>
…4243) Co-authored-by: Warren James <[email protected]> Co-authored-by: Neal Beeken <[email protected]> Co-authored-by: Bailey Pearson <[email protected]>
}); | ||
} | ||
|
||
export function isCSOTTimeoutContext( |
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.
Having a method that narrows the base abstraction to a concrete subtype as a required method on the base abstraction breaks the base abstraction. I've moved this to a floating function to keep the abstraction clean.
@@ -180,20 +180,14 @@ export abstract class TimeoutContext { | |||
|
|||
abstract get maxTimeMS(): number | null; | |||
|
|||
abstract get serverSelectionTimeout(): Timeout | null; | |||
abstract get timeoutForServerSelection(): Timeout | null; |
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.
half of our timeout methods were timeoutFor<x>
and the other half were <x>Timeout
. I've aligned them all to timeoutFor<x>
.
c3f31da
to
4fd4b24
Compare
…ver selection### Description
What is changing?
Is there new documentation needed for these changes?
What is the motivation for this change?
Release Highlight
Fill in title or leave empty for no highlight
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript