-
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?
NXDRIVE-2967: Upgrade the deprecated datetime adapter.. #5445
Conversation
* Bump platformdirs from 4.2.2 to 4.3.6 in /tools/deps Bumps [platformdirs](https://github.com/tox-dev/platformdirs) from 4.2.2 to 4.3.6. - [Release notes](https://github.com/tox-dev/platformdirs/releases) - [Changelog](https://github.com/tox-dev/platformdirs/blob/main/CHANGES.rst) - [Commits](tox-dev/platformdirs@4.2.2...4.3.6) --- updated-dependencies: - dependency-name: platformdirs dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Bump faker from 22.0.0 to 29.0.0 in /tools/deps (#5271) Bumps [faker](https://github.com/joke2k/faker) from 22.0.0 to 29.0.0. * Bump build from 1.2.1 to 1.2.2 in /tools/deps (#5274) Bumps [build](https://github.com/pypa/build) from 1.2.1 to 1.2.2. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]>
* Bump watchdog from 3.0.0 to 5.0.2 in /tools/deps Bumps [watchdog](https://github.com/gorakhargosh/watchdog) from 3.0.0 to 5.0.2. - [Release notes](https://github.com/gorakhargosh/watchdog/releases) - [Changelog](https://github.com/gorakhargosh/watchdog/blob/master/changelog.rst) - [Commits](gorakhargosh/watchdog@v3.0.0...v5.0.2) --- updated-dependencies: - dependency-name: watchdog dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> * Bump mypy from 1.10.0 to 1.11.2 in /tools/deps (#5207) Bumps [mypy](https://github.com/python/mypy) from 1.10.0 to 1.11.2. * Bump responses from 0.24.1 to 0.25.3 in /tools/deps (#5006) Bumps [responses](https://github.com/getsentry/responses) from 0.24.1 to 0.25.3. * Bump python-dateutil from 2.8.2 to 2.9.0.post0 in /tools/deps (#4680) Bumps [python-dateutil](https://github.com/dateutil/dateutil) from 2.8.2 to 2.9.0.post0. * Bump types-python-dateutil in /tools/deps (#5230) Bumps [types-python-dateutil](https://github.com/python/typeshed) from 2.8.19.20240106 to 2.9.0.20240906. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]>
* Bump pyobjc-framework-cocoa from 10.1 to 10.3.1 in /tools/deps Bumps [pyobjc-framework-cocoa](https://github.com/ronaldoussoren/pyobjc) from 10.1 to 10.3.1. - [Release notes](https://github.com/ronaldoussoren/pyobjc/releases) - [Changelog](https://github.com/ronaldoussoren/pyobjc/blob/master/docs/changelog.rst) - [Commits](ronaldoussoren/pyobjc@v10.1...v10.3.1) --- updated-dependencies: - dependency-name: pyobjc-framework-cocoa dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Bump pyobjc-framework-scriptingbridge from 10.1 to 10.3.1 in /tools/deps (#4987) Bumps [pyobjc-framework-scriptingbridge](https://github.com/ronaldoussoren/pyobjc) from 10.1 to 10.3.1. * Bump pyobjc-core from 10.1 to 10.3.1 in /tools/deps (#4990) Bumps [pyobjc-core](https://github.com/ronaldoussoren/pyobjc) from 10.1 to 10.3.1. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]>
) * Bump pyobjc-framework-fsevents from 10.1 to 10.3.1 in /tools/deps Bumps [pyobjc-framework-fsevents](https://github.com/ronaldoussoren/pyobjc) from 10.1 to 10.3.1. - [Release notes](https://github.com/ronaldoussoren/pyobjc/releases) - [Changelog](https://github.com/ronaldoussoren/pyobjc/blob/master/docs/changelog.rst) - [Commits](ronaldoussoren/pyobjc@v10.1...v10.3.1) --- updated-dependencies: - dependency-name: pyobjc-framework-fsevents dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Bump pyobjc-framework-coreservices from 10.1 to 10.3.1 in /tools/deps (#4986) Bumps [pyobjc-framework-coreservices](https://github.com/ronaldoussoren/pyobjc) from 10.1 to 10.3.1. * Bump pyobjc-framework-systemconfiguration in /tools/deps (#4989) Bumps [pyobjc-framework-systemconfiguration](https://github.com/ronaldoussoren/pyobjc) from 10.1 to 10.3.1. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]>
* Update deploy_ci_agent.sh
* Bump send2trash from 1.7.1 to 1.8.3 in /tools/deps Bumps [send2trash](https://github.com/arsenetar/send2trash) from 1.7.1 to 1.8.3. - [Release notes](https://github.com/arsenetar/send2trash/releases) - [Changelog](https://github.com/arsenetar/send2trash/blob/master/CHANGES.rst) - [Commits](https://github.com/arsenetar/send2trash/commits) --- updated-dependencies: - dependency-name: send2trash dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Bump altgraph from 0.17 to 0.17.4 in /tools/deps (#4565) Bumps [altgraph](https://github.com/ronaldoussoren/altgraph) from 0.17 to 0.17.4. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]>
* Bump requests from 2.31.0 to 2.32.3 in /tools/deps Bumps [requests](https://github.com/psf/requests) from 2.31.0 to 2.32.3. - [Release notes](https://github.com/psf/requests/releases) - [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md) - [Commits](psf/requests@v2.31.0...v2.32.3) --- updated-dependencies: - dependency-name: requests dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Bump future from 0.18.3 to 1.0.0 in /tools/deps (#4693) Bumps [future](https://github.com/PythonCharmers/python-future) from 0.18.3 to 1.0.0. * Bump pefile from 2023.2.7 to 2024.8.26 in /tools/deps (#5191) Bumps [pefile](https://github.com/erocarrera/pefile) from 2023.2.7 to 2024.8.26. * Bump pycparser from 2.21 to 2.22 in /tools/deps (#4753) Bumps [pycparser](https://github.com/eliben/pycparser) from 2.21 to 2.22. --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]>
* NXDRIVE-2925: Ignore zero-byte files
* Bump black from 23.12.1 to 24.10.0 in /tools/deps Bumps [black](https://github.com/psf/black) from 23.12.1 to 24.10.0. - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/main/CHANGES.md) - [Commits](psf/black@23.12.1...24.10.0) --- updated-dependencies: - dependency-name: black dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pooja Ramkrishna Daine <[email protected]> Co-authored-by: unknown <[email protected]>
…st_datetime_adapter_new
Reviewer's Guide by SourceryThis PR updates several dependencies to their latest versions and fixes timezone-related issues in datetime handling. The main changes include upgrading Python packages, addressing security vulnerabilities, and implementing a fix for zero-byte file handling. Sequence diagram for zero-byte file handlingsequenceDiagram
participant FoldersDialog
participant Path
FoldersDialog->>Path: get_size(file_path)
Path-->>FoldersDialog: file_size
alt file_size == 0
FoldersDialog->>FoldersDialog: Ignore zero-byte file
else file_size > 0
FoldersDialog->>FoldersDialog: Add file to paths
end
Class diagram for updated datetime handlingclassDiagram
class AutoRetryCursor {
+adapt_datetime_iso(val)
+execute(sql: str, parameters: Iterable[Any])
}
note for AutoRetryCursor "Updated to handle datetime with timezone awareness using adapt_datetime_iso method"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @gitofanindya - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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 comment
The 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.
@@ -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: |
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 (bug_risk): Return type hint is incorrect - method returns an integer not a boolean
@@ -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: |
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 (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.
if self.get_size(file_path) == 0: | |
if size == 0: |
except (ValueError, OverflowError, OSError) as e: | ||
log.warning( | ||
f"{e} file path: {os_path}. st_mtime value: {stat_info.st_mtime}" | ||
) | ||
mtime = datetime.utcfromtimestamp(0) | ||
mtime = datetime.fromtimestamp(0, tz=timezone.utc) |
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.
mtime = datetime.fromtimestamp(0, tz=timezone.utc) | |
EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) | |
mtime = EPOCH |
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 comment
The 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## wip-NXDRIVE-2929-upgrade-python-from-3.9.5-to-3.12.3 #5445 +/- ##
========================================================================================
+ Coverage 50.58% 51.98% +1.40%
========================================================================================
Files 96 96
Lines 16109 16121 +12
========================================================================================
+ Hits 8148 8381 +233
+ Misses 7961 7740 -221
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Summary by Sourcery
Upgrade dependencies and improve datetime handling. Update the macOS release workflow to use a .p12 certificate format. Remove deprecated datetime adapter warnings from tests.
Enhancements:
CI:
Documentation:
Tests: