-
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 all commits
c75e217
106e3a6
405f6d9
094f4de
9aa9228
4a40d03
c5b58eb
c0fac40
9805a47
b4b5d33
d898aa3
ed3221c
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 |
---|---|---|
@@ -1,6 +1,8 @@ | ||
""" | ||
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 | ||
|
@@ -18,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=}", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -838,9 +838,7 @@ def resume_transfer( | |
meth = ( | ||
self.dao.get_download | ||
if nature == "download" | ||
else self.dao.get_dt_upload | ||
if is_direct_transfer | ||
else self.dao.get_upload | ||
else self.dao.get_dt_upload if is_direct_transfer else self.dao.get_upload | ||
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. suggestion: Nested ternary operator could be simplified for better readability Consider using an if/elif/else block to make the logic more explicit and easier to maintain. else:
if is_direct_transfer:
meth = self.dao.get_dt_upload
else:
meth = self.dao.get_upload |
||
) | ||
func = partial(meth, uid=uid) # type: ignore | ||
self._resume_transfers(nature, func, is_direct_transfer=is_direct_transfer) | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -624,6 +624,9 @@ | |||||
txt += f" (+{self.overall_count - 1:,})" | ||||||
return txt | ||||||
|
||||||
def get_size(self, file_path: Path) -> bool: | ||||||
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 (bug_risk): Return type hint is incorrect - method returns an integer not a boolean |
||||||
return file_path.stat().st_size | ||||||
|
||||||
def _process_additionnal_local_paths(self, paths: List[str], /) -> None: | ||||||
"""Append more local paths to the upload queue.""" | ||||||
for local_path in paths: | ||||||
|
@@ -647,10 +650,17 @@ | |||||
# Save the path | ||||||
if path.is_dir(): | ||||||
for file_path, size in get_tree_list(path): | ||||||
if self.get_size(file_path) == 0: | ||||||
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. suggestion (performance): Redundant size check - file size is already available from get_tree_list() Consider using the size value already provided by get_tree_list() instead of calling get_size() again.
Suggested change
|
||||||
# ignoring zero byte files [NXDRIVE-2925] | ||||||
continue | ||||||
self.paths[file_path] = size | ||||||
else: | ||||||
try: | ||||||
self.paths[path] = path.stat().st_size | ||||||
file_size = self.get_size(path) | ||||||
if file_size == 0: | ||||||
# ignoring zero byte files [NXDRIVE-2925] | ||||||
continue | ||||||
self.paths[path] = file_size | ||||||
except OSError: | ||||||
log.warning(f"Error calling stat() on {path!r}", exc_info=True) | ||||||
continue | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
"""Cleanup old test users and workspaces.""" | ||
|
||
import env | ||
from nuxeo.client import Nuxeo | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
"""Collection of pytest markers to ease test filtering.""" | ||
|
||
import os | ||
|
||
import pytest | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
""" Common test utilities. """ | ||
|
||
import os | ||
import sys | ||
import tempfile | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
""" | ||
Test application Behavior. | ||
""" | ||
|
||
from nxdrive.behavior import Behavior | ||
|
||
from .. import ensure_no_exception | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
See NXDRIVE-742. | ||
""" | ||
|
||
import hashlib | ||
import os | ||
from pathlib import Path | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
""" | ||
Test pause/resume transfers in different scenarii. | ||
""" | ||
|
||
import re | ||
from unittest.mock import patch | ||
|
||
|
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.