chore: update Python to 3.14 and invenio deps#4058
Open
PascalRepond wants to merge 1 commit intorero:stagingfrom
Open
chore: update Python to 3.14 and invenio deps#4058PascalRepond wants to merge 1 commit intorero:stagingfrom
PascalRepond wants to merge 1 commit intorero:stagingfrom
Conversation
PascalRepond
force-pushed
the
rep-invenio-update
branch
8 times, most recently
from
481d2a4 to
e6ef4d3
Compare
PascalRepond
force-pushed
the
rep-invenio-update
branch
8 times, most recently
from
ff9f772 to
8e9df6b
Compare
PascalRepond
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBumped project runtime to Python 3.14, loosened/simplified dependency constraints, migrated timezone handling from pytz/timezone.utc to stdlib UTC/zoneinfo, applied PEP 604 unions, and made localized refactors to parsing, identity enrichment, a descriptor, form validation, and many tests/migrations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
PascalRepond
force-pushed
the
rep-invenio-update
branch
from
8e9df6b to
52df2c9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
PascalRepond
force-pushed
the
rep-invenio-update
branch
2 times, most recently
from
0d8ea0f to
bf02f80
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Update Python version to >=3.14.0.
Update invenio dependencies to their latest major versions.
Add max_retries and retry_on_timeout to elasticsearch client config for
better resilience in CI tests.
Adapt code for breaking changes with new Python and invenio versions:
- timezone: replace pytz with stdlib datetime.UTC and
zoneinfo.ZoneInfo; replace deprecated datetime.utcnow() with
datetime.now(UTC); remove pytz.utc.localize() calls since invenio
records now return timezone-aware datetimes
- Replace `tz.localize()` calls with `dt.astimezone(tz)` followed by
replace on time components, ensuring DST offsets are applied correctly.
- Convert datetimes to UTC before subtraction in loan duration
calculations: unlike pytz (which creates distinct tzinfo objects per
DST state), `ZoneInfo` reuses a single object, causing Python to
perform naive subtraction across DST boundaries and yielding incorrect
durations. Apply the same astimezone-then-replace pattern to overdue
fee calculation and loan age computation.
- permissions: add RoleNeed(role.name) to identity since
flask-security-invenio 4.x now uses RoleNeed(role.id)
- webargs: create a local FlaskParser with location="json_or_form" in
accounts_views.py instead of importing use_args/use_kwargs from
invenio_accounts (which is locked to location="form"). Cause:
webargs 5→8 made FlaskParser(location="form") strict; our JSON
requests were rejected before reaching view logic. Use
invenio-accounts' FlaskParser so that validation errors return a
400 status with RESTValidationError instead of webargs' default 422.
- marshmallow: remove context={"request": request} parameter from
Schema instantiation (no longer supported)
- classmethod+property: replace stacked @classmethod/@Property
decorators (removed in Python 3.13) with descriptors
(files/services.py) and inline computation (users/api.py)
- typing: replace typing.Optional with PEP 604 X | None syntax
- Override the default EmailField with a StringField to allow login with
both username and email, bypassing HTML5 and WTForms email validation.
Co-Authored-by: Pascal Repond <pascal.repond@rero.ch>
PascalRepond
force-pushed
the
rep-invenio-update
branch
from
bf02f80 to
f6d5590
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
rero_ils/modules/libraries/api.py (3)
192-204:⚠️ Potential issue | 🟠 MajorHandle repeating and multi-day open exceptions in
_has_is_open().This branch only checks the initial
start_date. An open exception that spans into future days, or one withrepeat, is treated as absent once its first occurrence is in the past, sonext_open()can raiseLibraryNeverOpeneven though future open days still exist.🐛 Proposed fix
- current_timestamp = datetime.now(UTC) + current_date = datetime.now(UTC).date() for exception_date in filter(lambda d: d["is_open"], self.get("exception_dates", [])): start_date = date_string_to_utc(exception_date["start_date"]) + end_date = ( + date_string_to_utc(exception_date["end_date"]) + if exception_date.get("end_date") + else start_date + ) + if exception_date.get("repeat"): + return True # avoid next_open infinite loop if an open exception date is # in the past - if start_date > current_timestamp: + if end_date.date() > current_date: return True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rero_ils/modules/libraries/api.py` around lines 192 - 204, The _has_is_open method only checks exception_dates' start_date and misses multi-day or repeating open exceptions; update _has_is_open to treat an exception as future-open if its span or any of its repeated occurrences fall after now: for each exception in exception_dates (from self.get("exception_dates", [])) convert start_date and end_date (or compute end from duration) via date_string_to_utc and if end_date > now return True, and if the exception has a repeat rule, compute the next occurrence(s) using the same repeat logic used by next_open (or a shared helper) to determine if any future occurrence or span exists—ensure you reference and reuse date_string_to_utc, _has_is_open, and the repeat handling used by next_open to avoid duplicating logic and to prevent the infinite-loop guard from wrongly excluding future open days.
341-362:⚠️ Potential issue | 🟠 MajorNormalize
get_open_days()to library-local calendar days first.This method is day-based, but it iterates on full timestamps. A same-day window like
09:00→18:00currently runs twice, andcount_open()inherits the extra day.🐛 Proposed fix
if isinstance(start_date, str): start_date = date_string_to_utc(start_date) if isinstance(end_date, str): end_date = date_string_to_utc(end_date) + if start_date.tzinfo is None: + start_date = start_date.replace(tzinfo=UTC) + if end_date.tzinfo is None: + end_date = end_date.replace(tzinfo=UTC) + tz = self.get_timezone() + current_date = start_date + current_day = start_date.astimezone(tz).date() + end_day = end_date.astimezone(tz).date() dates = [] - end_date += timedelta(days=1) - while end_date > start_date: - if self.is_open(date=start_date, day_only=True, date_only=True): - dates.append(start_date) - start_date += timedelta(days=1) + while current_day <= end_day: + if self.is_open(date=current_date, day_only=True, date_only=True): + dates.append(current_date) + current_date += timedelta(days=1) + current_day = current_date.astimezone(tz).date() return dates🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rero_ils/modules/libraries/api.py` around lines 341 - 362, get_open_days iterates timestamps and can count the same calendar day twice; normalize inputs to library-local calendar days first: convert start_date/end_date (strings via date_string_to_utc if needed) to the start of their calendar day (midnight) in the library timezone and set end_date to the start of the day after the end_date to make the loop day-based and inclusive; then iterate by whole days and call self.is_open(date=current_day, day_only=True, date_only=True). Update count_open to keep using len(self.get_open_days(...)) unchanged.
324-339:⚠️ Potential issue | 🟠 Major
ensure=Trueshould prefer exception hours.After the search loop,
datemay be open because of an exception or with exception-specific hours._get_opening_hour_by_day()only reads regularopening_hours, so this path can return the wrong time or blow up onNone.🐛 Proposed fix
if not ensure: return date - opening_hour = self._get_opening_hour_by_day(date.strftime("%A")) + opening_hour = None + for exception in self._get_exceptions_matching_date(date, day_only=True): + if exception["is_open"] and exception.get("times"): + opening_hour = exception["times"][0]["start_time"] + break + if opening_hour is None: + opening_hour = self._get_opening_hour_by_day(date.strftime("%A")) + if opening_hour is None: + return date time = [int(part) for part in opening_hour.split(":")]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rero_ils/modules/libraries/api.py` around lines 324 - 339, The next_open method can return a wrong time when the found date is open due to an exception because it unconditionally calls _get_opening_hour_by_day (which only reads regular opening_hours) and may return None; update next_open to determine the actual opening time for that specific date by checking for exception hours first (using the same logic is_open uses for exceptions) and only falling back to _get_opening_hour_by_day for regular weekdays, and ensure you handle a None opening hour safely (raise a clear error or skip to next day) so ensure=True never crashes when exception-specific hours exist; reference next_open, is_open, and _get_opening_hour_by_day when locating the logic to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 89-90: Update the urllib3 dependency spec to include a safe
minimum: replace the existing "urllib3<2.0.0" entry with
"urllib3>=1.26.20,<2.0.0" in pyproject.toml (locate the string "urllib3<2.0.0")
so pre-CVE-2024-37891 vulnerable releases are excluded while maintaining
Elasticsearch 7 compatibility.
---
Outside diff comments:
In `@rero_ils/modules/libraries/api.py`:
- Around line 192-204: The _has_is_open method only checks exception_dates'
start_date and misses multi-day or repeating open exceptions; update
_has_is_open to treat an exception as future-open if its span or any of its
repeated occurrences fall after now: for each exception in exception_dates (from
self.get("exception_dates", [])) convert start_date and end_date (or compute end
from duration) via date_string_to_utc and if end_date > now return True, and if
the exception has a repeat rule, compute the next occurrence(s) using the same
repeat logic used by next_open (or a shared helper) to determine if any future
occurrence or span exists—ensure you reference and reuse date_string_to_utc,
_has_is_open, and the repeat handling used by next_open to avoid duplicating
logic and to prevent the infinite-loop guard from wrongly excluding future open
days.
- Around line 341-362: get_open_days iterates timestamps and can count the same
calendar day twice; normalize inputs to library-local calendar days first:
convert start_date/end_date (strings via date_string_to_utc if needed) to the
start of their calendar day (midnight) in the library timezone and set end_date
to the start of the day after the end_date to make the loop day-based and
inclusive; then iterate by whole days and call self.is_open(date=current_day,
day_only=True, date_only=True). Update count_open to keep using
len(self.get_open_days(...)) unchanged.
- Around line 324-339: The next_open method can return a wrong time when the
found date is open due to an exception because it unconditionally calls
_get_opening_hour_by_day (which only reads regular opening_hours) and may return
None; update next_open to determine the actual opening time for that specific
date by checking for exception hours first (using the same logic is_open uses
for exceptions) and only falling back to _get_opening_hour_by_day for regular
weekdays, and ensure you handle a None opening hour safely (raise a clear error
or skip to next day) so ensure=True never crashes when exception-specific hours
exist; reference next_open, is_open, and _get_opening_hour_by_day when locating
the logic to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd2c31fe-9ebf-47e4-98dd-46f6018fe714
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (77)
.github/workflows/continuous-integration-test.ymlCLAUDE.mdDockerfile.baseINSTALL.mdpyproject.tomlrero_ils/accounts_views.pyrero_ils/alembic/2b0af71048a7_add_request_expiration_date.pyrero_ils/modules/acquisition/acq_orders/api.pyrero_ils/modules/api.pyrero_ils/modules/api_harvester/models.pyrero_ils/modules/cli/fixtures.pyrero_ils/modules/collections/api.pyrero_ils/modules/commons/identifiers.pyrero_ils/modules/documents/dumpers/indexer.pyrero_ils/modules/documents/serializers/ris.pyrero_ils/modules/documents/views.pyrero_ils/modules/entities/local_entities/indexer.pyrero_ils/modules/entities/remote_entities/replace.pyrero_ils/modules/ext.pyrero_ils/modules/files/services.pyrero_ils/modules/holdings/api.pyrero_ils/modules/holdings/cli.pyrero_ils/modules/holdings/utils.pyrero_ils/modules/ill_requests/api.pyrero_ils/modules/items/api/api.pyrero_ils/modules/items/api/circulation.pyrero_ils/modules/items/api/issue.pyrero_ils/modules/items/api/record.pyrero_ils/modules/items/utils.pyrero_ils/modules/libraries/api.pyrero_ils/modules/loans/api.pyrero_ils/modules/loans/cli.pyrero_ils/modules/loans/extensions.pyrero_ils/modules/loans/logs/api.pyrero_ils/modules/loans/tasks.pyrero_ils/modules/loans/utils.pyrero_ils/modules/migrations/api.pyrero_ils/modules/migrations/data/api.pyrero_ils/modules/migrations/data/views.pyrero_ils/modules/monitoring/api.pyrero_ils/modules/notifications/api.pyrero_ils/modules/notifications/tasks.pyrero_ils/modules/operation_logs/extensions.pyrero_ils/modules/patron_transaction_events/api.pyrero_ils/modules/patron_transactions/utils.pyrero_ils/modules/selfcheck/api.pyrero_ils/modules/selfcheck/utils.pyrero_ils/modules/serializers/base.pyrero_ils/modules/stats/views.pyrero_ils/modules/users/api.pyrero_ils/modules/utils.pyrero_ils/query.pyrero_ils/theme/forms.pyscripts/testtests/api/circulation/test_actions_views_checkin.pytests/api/circulation/test_borrow_limits.pytests/api/circulation/test_library_with_no_circulation.pytests/api/holdings/test_provisional_items.pytests/api/items/test_items_rest.pytests/api/loans/test_loans_rest.pytests/api/notifications/test_notifications_rest.pytests/api/selfcheck/test_selfcheck.pytests/api/test_commons_api.pytests/api/test_tasks.pytests/conftest.pytests/fixtures/basics.pytests/fixtures/circulation.pytests/migrations/ui/test_migrations_cli.pytests/ui/circulation/test_actions_auto_extend.pytests/ui/circulation/test_actions_expired_request.pytests/ui/circulation/test_actions_extend.pytests/ui/circulation/test_extend_external.pytests/ui/libraries/test_libraries_api.pytests/ui/loans/test_loans_api.pytests/unit/conftest.pytests/unit/documents/test_documents_dojson_marc21.pytests/utils.py
✅ Files skipped from review due to trivial changes (34)
- rero_ils/modules/collections/api.py
- INSTALL.md
- Dockerfile.base
- rero_ils/modules/items/api/issue.py
- rero_ils/modules/loans/cli.py
- rero_ils/modules/migrations/data/views.py
- rero_ils/modules/serializers/base.py
- rero_ils/modules/loans/tasks.py
- rero_ils/modules/loans/logs/api.py
- rero_ils/modules/patron_transaction_events/api.py
- rero_ils/modules/migrations/data/api.py
- rero_ils/modules/holdings/api.py
- .github/workflows/continuous-integration-test.yml
- rero_ils/modules/holdings/utils.py
- tests/api/circulation/test_borrow_limits.py
- rero_ils/query.py
- rero_ils/modules/entities/local_entities/indexer.py
- tests/fixtures/basics.py
- rero_ils/modules/ill_requests/api.py
- tests/api/holdings/test_provisional_items.py
- rero_ils/modules/items/api/circulation.py
- rero_ils/modules/notifications/tasks.py
- rero_ils/modules/notifications/api.py
- rero_ils/modules/selfcheck/api.py
- rero_ils/modules/items/utils.py
- rero_ils/modules/documents/dumpers/indexer.py
- tests/ui/loans/test_loans_api.py
- tests/ui/circulation/test_actions_extend.py
- tests/api/items/test_items_rest.py
- tests/unit/conftest.py
- rero_ils/modules/operation_logs/extensions.py
- rero_ils/modules/commons/identifiers.py
- tests/fixtures/circulation.py
- rero_ils/modules/patron_transactions/utils.py
🚧 Files skipped from review as they are similar to previous changes (28)
- rero_ils/modules/items/api/api.py
- rero_ils/modules/cli/fixtures.py
- tests/conftest.py
- rero_ils/modules/selfcheck/utils.py
- rero_ils/modules/entities/remote_entities/replace.py
- tests/api/circulation/test_library_with_no_circulation.py
- rero_ils/modules/documents/views.py
- rero_ils/modules/migrations/api.py
- rero_ils/modules/monitoring/api.py
- rero_ils/accounts_views.py
- rero_ils/modules/utils.py
- tests/api/test_tasks.py
- tests/migrations/ui/test_migrations_cli.py
- tests/api/test_commons_api.py
- rero_ils/modules/users/api.py
- tests/ui/libraries/test_libraries_api.py
- rero_ils/alembic/2b0af71048a7_add_request_expiration_date.py
- tests/api/circulation/test_actions_views_checkin.py
- tests/ui/circulation/test_extend_external.py
- rero_ils/modules/loans/utils.py
- tests/unit/documents/test_documents_dojson_marc21.py
- rero_ils/modules/api_harvester/models.py
- rero_ils/theme/forms.py
- tests/api/notifications/test_notifications_rest.py
- CLAUDE.md
- rero_ils/modules/api.py
- rero_ils/modules/acquisition/acq_orders/api.py
- rero_ils/modules/loans/api.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update Python version to >=3.14.0.
Update invenio dependencies to their latest major versions.
Add max_retries and retry_on_timeout to elasticsearch client config for
better resilience in CI tests.
Adapt code for breaking changes with new Python and invenio versions:
zoneinfo.ZoneInfo; replace deprecated datetime.utcnow() with
datetime.now(UTC); remove pytz.utc.localize() calls since invenio
records now return timezone-aware datetimes
tz.localize()calls withdt.astimezone(tz)followed byreplace on time components, ensuring DST offsets are applied correctly.
calculations: unlike pytz (which creates distinct tzinfo objects per
DST state),
ZoneInforeuses a single object, causing Python toperform naive subtraction across DST boundaries and yielding incorrect
durations. Apply the same astimezone-then-replace pattern to overdue
fee calculation and loan age computation.
flask-security-invenio 4.x now uses RoleNeed(role.id)
accounts_views.py instead of importing use_args/use_kwargs from
invenio_accounts (which is locked to location="form"). Cause:
webargs 5→8 made FlaskParser(location="form") strict; our JSON
requests were rejected before reaching view logic. Use
invenio-accounts' FlaskParser so that validation errors return a
400 status with RESTValidationError instead of webargs' default 422.
Schema instantiation (no longer supported)
decorators (removed in Python 3.13) with descriptors
(files/services.py) and inline computation (users/api.py)
both username and email, bypassing HTML5 and WTForms email validation.