Skip to content

PYTHON-5169 - Deprecate Hedged Reads option #2213

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 25, 2025
Merged

PYTHON-5169 - Deprecate Hedged Reads option #2213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d5aa363
PYTHON-5169 - Deprecate Hedged Reads option
NoahStapp Mar 24, 2025
396b076
Add test, address review
NoahStapp Mar 24, 2025
ce05c69
Address review
NoahStapp Mar 24, 2025
5b87911
Stacklevel 4 for constructor
NoahStapp Mar 25, 2025
c492a8f
Merge branch 'master' into PYTHON-5169
ShaneHarvey Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions pymongo/read_preferences.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from __future__ import annotations

import warnings
from collections import abc
from typing import TYPE_CHECKING, Any, Mapping, Optional, Sequence

Expand Down Expand Up @@ -103,6 +104,11 @@ def _validate_hedge(hedge: Optional[_Hedge]) -> Optional[_Hedge]:
if not isinstance(hedge, dict):
raise TypeError(f"hedge must be a dictionary, not {hedge!r}")

warnings.warn(
"Hedged reads are deprecated starting in server version 8.0.",
Copy link
Member

Choose a reason for hiding this comment

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

Mentioning the server side deprecation is good but we should first mention the driver side deprecation. Something like:
the read preference "hedge" option is deprecated in PyMongo 4.12+ because hedged reads are deprecated in MongoDB version 8.0+. Support for "hedge" will be removed in PyMongo 5.0.

DeprecationWarning,
stacklevel=2,
Copy link
Member

Choose a reason for hiding this comment

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

One last question. I'm wondering about the stacklevel here. Can you provide an example of the warning in the ReadPreference constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stacklevel=2:

  /Users/nstapp/Github/mongo-python-driver/pymongo/read_preferences.py:131: DeprecationWarning: The read preference 'hedge' option is deprecated in PyMongo 4.12+ because hedged reads are deprecated in MongoDB version 8.0+. Support for 'hedge' will be removed in PyMongo 5.0.
    self.__hedge = _validate_hedge(hedge)

stacklevel=4:

  /Users/nstapp/Github/mongo-python-driver/test/asynchronous/test_read_preferences.py:583: DeprecationWarning: The read preference 'hedge' option is deprecated in PyMongo 4.12+ because hedged reads are deprecated in MongoDB version 8.0+. Support for 'hedge' will be removed in PyMongo 5.0.
    cls(hedge={"enabled": True})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we want stacklevel=4 to provide a useful traceback.

Copy link
Member

Choose a reason for hiding this comment

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

Let's make it so.

)
return hedge


Expand Down Expand Up @@ -143,6 +149,11 @@ def document(self) -> dict[str, Any]:
if self.__max_staleness != -1:
doc["maxStalenessSeconds"] = self.__max_staleness
if self.__hedge not in (None, {}):
warnings.warn(
Copy link
Member

Choose a reason for hiding this comment

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

The document helper should not raise a warning. We already raised a warning in the constructor. We do need to keep the warning in hedge() since we will remove that api in 5.0.

"Hedged reads are deprecated starting in server version 8.0.",
DeprecationWarning,
stacklevel=2,
)
Copy link
Member

Choose a reason for hiding this comment

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

Let's update all the hedge param docs to say:

    :param hedge: **DEPRECATED** - The :attr:`~hedge` for this read preference.

doc["hedge"] = self.__hedge
return doc

Expand Down Expand Up @@ -203,6 +214,12 @@ def hedge(self) -> Optional[_Hedge]:

.. versionadded:: 3.11
"""
Copy link
Member

Choose a reason for hiding this comment

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

Let's update this docstring with **DEPRECATED** and include the explanation as well.

if self.__hedge is not None:
warnings.warn(
"Hedged reads are deprecated starting in server version 8.0.",
DeprecationWarning,
stacklevel=2,
)
return self.__hedge

@property
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ filterwarnings = [
"module:please use dns.resolver.Resolver.resolve:DeprecationWarning",
# https://github.com/dateutil/dateutil/issues/1314
"module:datetime.datetime.utc:DeprecationWarning",
"module:Hedged reads are deprecated:DeprecationWarning",
Copy link
Member

Choose a reason for hiding this comment

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

Rather than using this approach we should manually catch/silence hedge errors only in tests that are intended to use it. We should also add a new test that we do raise the warning when hedge is used.

]
markers = [
"auth_aws: tests that rely on pymongo-auth-aws",
Expand Down
Loading