Skip to content

Commit

Permalink
Error handling: Use ValueError exceptions instead of assert
Browse files Browse the repository at this point in the history
  • Loading branch information
amotl committed Nov 1, 2024
1 parent 62ccb1a commit 5512da0
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Unreleased
server stacktraces into exception messages.
- Refactoring: The module namespace ``crate.client.test_util`` has been
renamed to ``crate.testing.util``.
- Error handling: At two spots in cursor / value converter handling, where
``assert`` statements have been used, ``ValueError`` exceptions are raised
now.


.. _Migrate from crate.client to sqlalchemy-cratedb: https://cratedb.com/docs/sqlalchemy-cratedb/migrate-from-crate-client.html
.. _sqlalchemy-cratedb: https://pypi.org/project/sqlalchemy-cratedb/
Expand Down
16 changes: 9 additions & 7 deletions src/crate/client/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,11 @@ def _convert_rows(self):
"""
Iterate rows, apply type converters, and generate converted rows.
"""
assert ( # noqa: S101
"col_types" in self._result and self._result["col_types"]
), "Unable to apply type conversion without `col_types` information"
if not ("col_types" in self._result and self._result["col_types"]):
raise ValueError(

Check warning on line 226 in src/crate/client/cursor.py

View check run for this annotation

Codecov / codecov/patch

src/crate/client/cursor.py#L226

Added line #L226 was not covered by tests
"Unable to apply type conversion "
"without `col_types` information"
)

# Resolve `col_types` definition to converter functions. Running
# the lookup redundantly on each row loop iteration would be a
Expand Down Expand Up @@ -302,10 +304,10 @@ def _timezone_from_utc_offset(tz) -> timezone:
"""
UTC offset in string format (e.g. `+0530`) to `datetime.timezone`.
"""
# TODO: Remove use of `assert`. Better use exceptions?
assert ( # noqa: S101
len(tz) == 5
), f"Time zone '{tz}' is given in invalid UTC offset format"
if len(tz) != 5:
raise ValueError(
f"Time zone '{tz}' is given in invalid UTC offset format"
)
try:
hours = int(tz[:3])
minutes = int(tz[0] + tz[3:])
Expand Down
2 changes: 1 addition & 1 deletion tests/client/test_cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test_create_with_timezone_as_utc_offset_failure(self):
Verify the cursor trips when trying to use invalid UTC offset strings.
"""
connection = self.get_mocked_connection()
with self.assertRaises(AssertionError) as ex:
with self.assertRaises(ValueError) as ex:
connection.cursor(time_zone="foobar")
self.assertEqual(
str(ex.exception),
Expand Down

0 comments on commit 5512da0

Please sign in to comment.