PYTHON-5169 - Deprecate Hedged Reads option#2213
Conversation
pymongo/read_preferences.py
Outdated
| raise TypeError(f"hedge must be a dictionary, not {hedge!r}") | ||
|
|
||
| warnings.warn( | ||
| "Hedged reads are deprecated starting in server version 8.0.", |
There was a problem hiding this comment.
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.
pyproject.toml
Outdated
| "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", |
There was a problem hiding this comment.
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.
ShaneHarvey
left a comment
ShaneHarvey
left a comment
There was a problem hiding this comment.
Could you update the changelog as well?
| :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. |
There was a problem hiding this comment.
Can hedge also appear in the URI/MongoClient kwarg?
There was a problem hiding this comment.
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.
pymongo/read_preferences.py
Outdated
| "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, | ||
| ) |
There was a problem hiding this comment.
Let's update all the hedge param docs to say:
:param hedge: **DEPRECATED** - The :attr:`~hedge` for this read preference.
| @@ -203,6 +214,12 @@ def hedge(self) -> Optional[_Hedge]: | |||
|
|
|||
| .. versionadded:: 3.11 | |||
| """ | |||
There was a problem hiding this comment.
Let's update this docstring with **DEPRECATED** and include the explanation as well.
pymongo/read_preferences.py
Outdated
| if self.__max_staleness != -1: | ||
| doc["maxStalenessSeconds"] = self.__max_staleness | ||
| if self.__hedge not in (None, {}): | ||
| warnings.warn( |
There was a problem hiding this comment.
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.
pymongo/read_preferences.py
Outdated
| 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, |
There was a problem hiding this comment.
One last question. I'm wondering about the stacklevel here. Can you provide an example of the warning in the ReadPreference constructor?
There was a problem hiding this comment.
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})
There was a problem hiding this comment.
I think we want stacklevel=4 to provide a useful traceback.
3 participants
No description provided.