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 2 commits
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
6 changes: 6 additions & 0 deletions doc/changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ PyMongo 4.12 brings a number of changes including:
- Support for configuring DEK cache lifetime via the ``key_expiration_ms`` argument to
:class:`~pymongo.encryption_options.AutoEncryptionOpts`.
- Support for $lookup in CSFLE and QE supported on MongoDB 8.1+.
- Deprecated the ``hedge`` parameter for
:class:`~pymongo.read_preferences.PrimaryPreferred`,
:class:`~pymongo.read_preferences.Secondary`,
:class:`~pymongo.read_preferences.SecondaryPreferred`,
:class:`~pymongo.read_preferences.Nearest`. Support for ``hedge`` will be removed in PyMongo 5.0.
Copy link
Member

Choose a reason for hiding this comment

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

Can hedge also appear in the URI/MongoClient kwarg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not listed in our docs page, so I don't believe so. You can create a read preference instance and pass that as a kwarg, but that will still pass through the read preference constructor.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for checking



Issues Resolved
...............
Expand Down
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(
"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.

"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.

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(
"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,
)
return self.__hedge

@property
Expand Down
61 changes: 37 additions & 24 deletions test/asynchronous/test_read_preferences.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
)
from test.utils_shared import (
OvertCommandListener,
_ignore_deprecations,
async_wait_until,
one,
)
Expand Down Expand Up @@ -542,33 +543,44 @@ def test_read_preference_document_hedge(self):
for mode, cls in cases.items():
with self.assertRaises(TypeError):
cls(hedge=[]) # type: ignore

pref = cls(hedge={})
self.assertEqual(pref.document, {"mode": mode})
out = _maybe_add_read_preference({}, pref)
if cls == SecondaryPreferred:
# SecondaryPreferred without hedge doesn't add $readPreference.
self.assertEqual(out, {})
else:
with _ignore_deprecations():
pref = cls(hedge={})
self.assertEqual(pref.document, {"mode": mode})
out = _maybe_add_read_preference({}, pref)
if cls == SecondaryPreferred:
# SecondaryPreferred without hedge doesn't add $readPreference.
self.assertEqual(out, {})
else:
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge: dict[str, Any] = {"enabled": True}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge: dict[str, Any] = {"enabled": True}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))
hedge = {"enabled": False}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge = {"enabled": False}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))
hedge = {"enabled": False, "extra": "option"}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge = {"enabled": False, "extra": "option"}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))
def test_read_preference_hedge_deprecated(self):
cases = {
"primaryPreferred": PrimaryPreferred,
"secondary": Secondary,
"secondaryPreferred": SecondaryPreferred,
"nearest": Nearest,
}
for _, cls in cases.items():
with self.assertRaises(DeprecationWarning):
cls(hedge={"enabled": True})

async def test_send_hedge(self):
cases = {
Expand All @@ -582,7 +594,8 @@ async def test_send_hedge(self):
client = await self.async_rs_client(event_listeners=[listener])
await client.admin.command("ping")
for _mode, cls in cases.items():
pref = cls(hedge={"enabled": True})
with _ignore_deprecations():
pref = cls(hedge={"enabled": True})
coll = client.test.get_collection("test", read_preference=pref)
listener.reset()
await coll.find_one()
Expand Down
61 changes: 37 additions & 24 deletions test/test_read_preferences.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
)
from test.utils_shared import (
OvertCommandListener,
_ignore_deprecations,
one,
wait_until,
)
Expand Down Expand Up @@ -522,33 +523,44 @@ def test_read_preference_document_hedge(self):
for mode, cls in cases.items():
with self.assertRaises(TypeError):
cls(hedge=[]) # type: ignore

pref = cls(hedge={})
self.assertEqual(pref.document, {"mode": mode})
out = _maybe_add_read_preference({}, pref)
if cls == SecondaryPreferred:
# SecondaryPreferred without hedge doesn't add $readPreference.
self.assertEqual(out, {})
else:
with _ignore_deprecations():
pref = cls(hedge={})
self.assertEqual(pref.document, {"mode": mode})
out = _maybe_add_read_preference({}, pref)
if cls == SecondaryPreferred:
# SecondaryPreferred without hedge doesn't add $readPreference.
self.assertEqual(out, {})
else:
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge: dict[str, Any] = {"enabled": True}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge: dict[str, Any] = {"enabled": True}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))
hedge = {"enabled": False}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge = {"enabled": False}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))
hedge = {"enabled": False, "extra": "option"}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))

hedge = {"enabled": False, "extra": "option"}
pref = cls(hedge=hedge)
self.assertEqual(pref.document, {"mode": mode, "hedge": hedge})
out = _maybe_add_read_preference({}, pref)
self.assertEqual(out, SON([("$query", {}), ("$readPreference", pref.document)]))
def test_read_preference_hedge_deprecated(self):
cases = {
"primaryPreferred": PrimaryPreferred,
"secondary": Secondary,
"secondaryPreferred": SecondaryPreferred,
"nearest": Nearest,
}
for _, cls in cases.items():
with self.assertRaises(DeprecationWarning):
cls(hedge={"enabled": True})

def test_send_hedge(self):
cases = {
Expand All @@ -562,7 +574,8 @@ def test_send_hedge(self):
client = self.rs_client(event_listeners=[listener])
client.admin.command("ping")
for _mode, cls in cases.items():
pref = cls(hedge={"enabled": True})
with _ignore_deprecations():
pref = cls(hedge={"enabled": True})
coll = client.test.get_collection("test", read_preference=pref)
listener.reset()
coll.find_one()
Expand Down
Loading