Skip to content

PYTHON-2287 Improve error message for invalid boolean option #1236

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

Merged
merged 4 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion bson/codec_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ def __new__(
"subclass of collections.abc.MutableMapping"
)
if not isinstance(tz_aware, bool):
raise TypeError("tz_aware must be True or False")
raise TypeError(f"tz_aware must be True or False, was: tz_aware={tz_aware}")
if uuid_representation not in ALL_UUID_REPRESENTATIONS:
raise ValueError(
"uuid_representation must be a value from bson.binary.UuidRepresentation"
Expand Down
1 change: 1 addition & 0 deletions doc/contributors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,4 @@ The following is a list of people who have contributed to
- Jean-Christophe Fillion-Robin (jcfr)
- Sean Cheah (thalassemia)
- Dainis Gorbunovs (DainisGorbunovs)
- Iris Ho (sleepyStick)
9 changes: 1 addition & 8 deletions pymongo/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
from pymongo.read_concern import ReadConcern
from pymongo.read_preferences import _MONGOS_MODES, _ServerMode
from pymongo.server_api import ServerApi
from pymongo.write_concern import DEFAULT_WRITE_CONCERN, WriteConcern
from pymongo.write_concern import DEFAULT_WRITE_CONCERN, WriteConcern, validate_boolean

ORDERED_TYPES: Sequence[Type] = (SON, OrderedDict)

Expand Down Expand Up @@ -170,13 +170,6 @@ def raise_config_error(key: str, dummy: Any) -> NoReturn:
}


def validate_boolean(option: str, value: Any) -> bool:
"""Validates that 'value' is True or False."""
if isinstance(value, bool):
return value
raise TypeError(f"{option} must be True or False")


def validate_boolean_or_string(option: str, value: Any) -> bool:
"""Validates that value is True, False, 'true', or 'false'."""
if isinstance(value, str):
Expand Down
7 changes: 3 additions & 4 deletions pymongo/pyopenssl_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from pymongo.ocsp_support import _load_trusted_ca_certs, _ocsp_callback
from pymongo.socket_checker import SocketChecker as _SocketChecker
from pymongo.socket_checker import _errno_from_exception
from pymongo.write_concern import validate_boolean

try:
import certifi
Expand Down Expand Up @@ -228,8 +229,7 @@ def __get_check_hostname(self):
return self._check_hostname

def __set_check_hostname(self, value):
if not isinstance(value, bool):
raise TypeError("check_hostname must be True or False")
validate_boolean("check_hostname", value)
self._check_hostname = value

check_hostname = property(__get_check_hostname, __set_check_hostname)
Expand All @@ -238,8 +238,7 @@ def __get_check_ocsp_endpoint(self):
return self._callback_data.check_ocsp_endpoint

def __set_check_ocsp_endpoint(self, value):
if not isinstance(value, bool):
raise TypeError("check_ocsp must be True or False")
validate_boolean("check_ocsp", value)
self._callback_data.check_ocsp_endpoint = value

check_ocsp_endpoint = property(__get_check_ocsp_endpoint, __set_check_ocsp_endpoint)
Expand Down
14 changes: 10 additions & 4 deletions pymongo/write_concern.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
from pymongo.errors import ConfigurationError


# Moved here to avoid a circular import.
def validate_boolean(option: str, value: Any) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment above this that says:

# Moved here to avoid a circular import.

"""Validates that 'value' is True or False."""
if isinstance(value, bool):
return value
raise TypeError(f"{option} must be True or False, was: {option}={value}")


class WriteConcern:
"""WriteConcern

Expand Down Expand Up @@ -65,13 +73,11 @@ def __init__(
self.__document["wtimeout"] = wtimeout

if j is not None:
if not isinstance(j, bool):
raise TypeError("j must be True or False")
validate_boolean("j", j)
self.__document["j"] = j

if fsync is not None:
if not isinstance(fsync, bool):
raise TypeError("fsync must be True or False")
validate_boolean("fsync", fsync)
if j and fsync:
raise ConfigurationError("Can't set both j and fsync at the same time")
self.__document["fsync"] = fsync
Expand Down
7 changes: 7 additions & 0 deletions test/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@ def test_mongo_client(self):
self.assertEqual(direct, direct2)
self.assertFalse(direct != direct2)

def test_validate_boolean(self):
self.db.test.update_one({}, {"$set": {"total": 1}}, upsert=True)
with self.assertRaisesRegex(
TypeError, "upsert must be True or False, was: upsert={'upsert': True}"
):
self.db.test.update_one({}, {"$set": {"total": 1}}, {"upsert": True}) # type: ignore


if __name__ == "__main__":
unittest.main()