-
Notifications
You must be signed in to change notification settings - Fork 53
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
NXDRIVE-2967: Upgrade the deprecated datetime adapter.. #5445
base: wip-NXDRIVE-2929-upgrade-python-from-3.9.5-to-3.12.3
Are you sure you want to change the base?
Changes from 10 commits
b4b5d33
d898aa3
ed3221c
3ebc1c6
36c10af
4200605
69d0baf
dda1df2
16485de
569349a
1b9c085
997db2f
3461608
ef063c1
0582844
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
Query formatting in this file is based on http://www.sqlstyle.guide/ | ||
""" | ||
|
||
from datetime import datetime, timezone | ||
import sys | ||
from contextlib import suppress | ||
from logging import getLogger | ||
|
@@ -19,14 +20,20 @@ | |
|
||
log = getLogger(__name__) | ||
|
||
|
||
class AutoRetryCursor(Cursor): | ||
def adapt_datetime_iso(self, val): | ||
return datetime.fromtimestamp(val.strftime('%s'), tz=timezone.utc) | ||
def execute(self, sql: str, parameters: Iterable[Any] = ()) -> Cursor: | ||
count = 1 | ||
while True: | ||
count += 1 | ||
try: | ||
return super().execute(sql, parameters) | ||
import sqlite3 | ||
# return super().execute(sql, parameters) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: The datetime parameter conversion logic should be simplified and the original super().execute() call restored The current implementation with register_adapter could have unintended side effects. Consider moving the datetime conversion to a separate method and maintaining the original execution flow. |
||
# new_param = tuple( datetime.fromtimestamp(param, tz=timezone.utc) if isinstance(param, datetime) else param for param in parameters ) | ||
new_param = tuple( sqlite3.register_adapter(param, self.adapt_datetime_iso) if isinstance(param, datetime) else param for param in parameters ) | ||
|
||
return super().execute(sql, new_param) | ||
except OperationalError as exc: | ||
log.info( | ||
f"Retry locked database #{count}, {sql=}, {parameters=}", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Error handling should use consistent timezone approach
Both the try and except blocks now use timezone-aware timestamps, but the error handling could be more consistent by using a common approach or constant for the fallback time.