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

GODRIVER-2495 Undeprecate legacy timeouts. #1026

Merged
merged 2 commits into from Jul 19, 2022
Merged

GODRIVER-2495 Undeprecate legacy timeouts. #1026

merged 2 commits into from Jul 19, 2022

Conversation

benjirewis
Copy link
Contributor

GODRIVER-2495

Changes Deprecated: notices for legacy timeouts to NOTE sections suggesting using Timeout in place of the given legacy timeout option.

Deprecating legacy timeout options with the addition of Timeout to ClientOptions has left some users with static analysis failures (deprecation warnings) and no stable alternative (as Timeout is still provisional API). We should wait until CSOT is more stable before doing GODRIVER-2342.

Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

Looks good with a proposed rewording.

// Deprecated: This option is deprecated and will eventually be removed in version 2.0 of the driver. The more general
// Timeout option should be used in its place to control the amount of time that the Aggregate operation can run before
// returning an error. MaxTime is still usable through the deprecated setter.
// NOTE(benjirewis): The use of MaxTime is discouraged. The more general Timeout option should be used in its place
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// NOTE(benjirewis): The use of MaxTime is discouraged. The more general Timeout option should be used in its place
// NOTE(benjirewis): MaxTime will be deprecated in a future release. The more general Timeout option may be used in its place

Until Timeout is documented as stable, I do not think users should be discouraged from using other timeouts. The behavior of Timeout is subject to change.

Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@benjirewis benjirewis merged commit ee4f97f into mongodb:master Jul 19, 2022
@benjirewis benjirewis deleted the undeprecateTimeouts branch July 19, 2022 17:00
benjirewis added a commit that referenced this pull request Jul 19, 2022
Uses NOTEs to suggest that users use Timeout instead of legacy timeout options as opposed to deprecating legacy timeout options.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants