-
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
Test dt adapter #5524
base: wip-NXDRIVE-2929-upgrade-python-from-3.9.5-to-3.12.3
Are you sure you want to change the base?
Test dt adapter #5524
Changes from all commits
c75e217
106e3a6
405f6d9
094f4de
9aa9228
4a40d03
c5b58eb
c0fac40
9805a47
b4b5d33
d898aa3
ed3221c
3ebc1c6
36c10af
4200605
69d0baf
dda1df2
16485de
569349a
1b9c085
997db2f
3461608
59208b8
ad685a0
55e58fa
d31181b
a19ab90
92f345f
4bdd34d
b32ba20
3657983
9800c7b
bebf0ba
4f2536f
d2e136c
9d0ba23
a289429
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) | ||
# 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 ) | ||
Comment on lines
+32
to
+34
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): The datetime adapter registration approach could cause issues with concurrent database access Using register_adapter affects the entire SQLite connection. Consider using value adaptation at the parameter level instead. Also, the original code is still present in comments which should be cleaned up. |
||
|
||
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 | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -36,15 +36,15 @@ def dump(database: Path, dump_file: Path, /) -> None: | |||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
log.info(f"Dumping the database {database!r} into {dump_file!r}...") | ||||||||||||||||||||||||||||||||||
with sqlite3.connect(str(database)) as con, dump_file.open( | ||||||||||||||||||||||||||||||||||
mode="w", encoding="utf-8" | ||||||||||||||||||||||||||||||||||
) as f: | ||||||||||||||||||||||||||||||||||
con = sqlite3.connect(str(database)) | ||||||||||||||||||||||||||||||||||
with dump_file.open(mode="w", encoding="utf-8") as f: | ||||||||||||||||||||||||||||||||||
for line in con.iterdump(): | ||||||||||||||||||||||||||||||||||
f.write(f"{line}\n") | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
# Force write of file to disk | ||||||||||||||||||||||||||||||||||
f.flush() | ||||||||||||||||||||||||||||||||||
fsync(f.fileno()) | ||||||||||||||||||||||||||||||||||
con.close() | ||||||||||||||||||||||||||||||||||
Comment on lines
+39
to
+47
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: Use context manager pattern for database connection to ensure proper cleanup This ensures the connection is closed even if an exception occurs during the dump process.
Suggested change
Comment on lines
+39
to
+47
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: Use a context manager for database connection to ensure proper resource cleanup While the connection is being closed explicitly, using a context manager (with statement) would be safer and ensure the connection is closed even if an exception occurs. |
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
log.info("Dump finished with success.") | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -624,6 +624,9 @@ def _files_display(self) -> str: | |
txt += f" (+{self.overall_count - 1:,})" | ||
return txt | ||
|
||
def get_size(self, file_path: Path) -> bool: | ||
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 @@ def _process_additionnal_local_paths(self, paths: List[str], /) -> None: | |
# Save the path | ||
if path.is_dir(): | ||
for file_path, size in get_tree_list(path): | ||
if self.get_size(file_path) == 0: | ||
# 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 | ||
Comment on lines
+653
to
+662
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: Consider extracting the zero-byte file check into a helper method to avoid code duplication The zero-byte file check logic is repeated. Consider creating a helper method like _should_skip_file(path) to handle this check in both cases. Suggested implementation: def get_size(self, file_path: Path) -> bool:
return file_path.stat().st_size
def _should_skip_file(self, file_path: Path) -> bool:
"""Check if file should be skipped (zero-byte files) [NXDRIVE-2925]."""
return self.get_size(file_path) == 0
if self._should_skip_file(file_path):
continue file_size = self.get_size(path)
if self._should_skip_file(path):
continue
self.paths[path] = file_size |
||
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 | ||
|
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.
issue: The strftime('%s') format specifier is not guaranteed to work on all platforms, particularly Windows