Skip to content

refactor!: make all API and download datetimes timezone-aware UTC - #266

Open
mkb79 wants to merge 1 commit into
masterfrom
refactor/tz-aware-datetimes
Open

refactor!: make all API and download datetimes timezone-aware UTC#266
mkb79 wants to merge 1 commit into
masterfrom
refactor/tz-aware-datetimes

Conversation

@mkb79

@mkb79 mkb79 commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Follow-up to #265. That PR fixed the --start-date crash from #264 by tolerating both API timestamp shapes, but deliberately kept the parsed values naive so the change stayed contained. This PR does the refactor that was left out.

Why

Naive datetimes are the reason the two hardcoded formats could coexist unnoticed for so long: the trailing Z was parsed as a literal character and its meaning silently dropped. Once the timezone is actually carried, the "with or without fraction" distinction stops being a parsing concern at all.

What changed

Parse API timestamps with datetime.fromisoformat() instead of a list of formats. Timezone information is mandatory and the result is normalized to UTC — an offset other than Z denotes the same instant and is accepted, an ambiguous timestamp without any offset is rejected.

Comparing naive against aware datetimes raises TypeError, so this has to land atomically rather than site by site:

utils.to_utc_datetime() New. Normalizes to UTC, interpreting naive values as UTC — matching the --start-date/--end-date help texts, which already ask for a UTC date.
utils.UTCDateTime New click.DateTime subclass so datetime_type yields aware values. All its formats either end in a literal Z or carry no timezone, and both mean UTC here.
Library.from_api() Normalizes its start_date/end_date bounds, so programmatic callers passing naive values keep working.
models.py, exceptions.py, cmd_download.py datetime.utcnow()datetime.now(UTC), datetime.utcfromtimestamp()datetime.fromtimestamp(..., tz=UTC). Both deprecated since Python 3.12.
Goodreads plugin .replace(tzinfo=timezone.utc) dropped, since the parser now returns aware values.

⚠️ Breaking

parse_api_datetime() and datetime_type now return timezone-aware UTC datetimes. Plugins that compare those results against naive datetimes will raise TypeError and have to be adjusted. .isoformat() output gains a +00:00 suffix.

Verification

A harness targeting the naive/aware fault lines specifically — all passing:

  • parse_api_datetime: both API shapes, 7-digit fractions, +02:00 offsets; naive, invalid and None inputs rejected with ValueError
  • to_utc_datetime: naive → UTC, aware +02:00 → correctly converted
  • datetime_type: all five accepted formats yield aware UTC; already-parsed datetime objects are normalized too
  • Library.from_api: naive start_date (no TypeError), aware start_date in another timezone, end_date filtering; purchased_after still serialized as 2008-01-01T00:00:00.000000Z
  • is_published() and ItemNotPublished with both timestamp shapes
  • _reuse_voucher: expired refresh_dateVoucherNeedRefresh, expired ExpiresDownloadUrlExpired, valid voucher accepted

Also: ruff check src plugin_cmds 432 → 431 findings, none new; importing the touched modules under -W error::DeprecationWarning is clean.

Out of scope

A review pass turned up four pre-existing bugs in code adjacent to this change. All four reproduce identically on master with the refactor stashed, so they are not regressions and are deliberately left for separate PRs:

  1. Podcast catalog fallback bypasses date filtering. When episode_count != len(children), get_child_items() passes the original request_params — including start_date/end_date — to Catalog.from_api(), which forwards them to catalog/products as query parameters and returns the result without any local date filtering.
  2. AttributeError on partial library responses. With any date bound set, an item with purchase_date: null and a missing or null library_status hits .get("date_added") on None.
  3. TypeError on a null refresh_date. _reuse_voucher() checks "refresh_date" in content_license and then converts the value, so an explicit null reaches strptime as None.
  4. is_published() returns None when publication_datetime is missing. Download paths read that as "not published" and then pass None into ItemNotPublished, which now raises ValueError from the parser.

Unrelated, carried over from #265: --end-date 2019-11-29 is parsed as 00:00:00, so "on or before this UTC date" still means the start of that day.

Follow-up to #265, which fixed the `--start-date` crash by tolerating both
API timestamp shapes but kept the parsed values naive. Naive datetimes were
the reason the two hardcoded formats could coexist for so long: the `Z` was
parsed as a literal and its meaning silently dropped.

Parse API timestamps with `datetime.fromisoformat()` instead of a list of
formats. Timezone information is now mandatory and the result is normalized
to UTC, so an offset other than `Z` denotes the same instant and is accepted,
while an ambiguous timestamp without any offset is rejected.

Because comparing naive against aware datetimes raises `TypeError`, this has
to happen atomically rather than site by site:

* `utils.to_utc_datetime()` normalizes a datetime to UTC, interpreting naive
  values as UTC. That matches the `--start-date` / `--end-date` help texts,
  which already asked for a UTC date.
* `utils.UTCDateTime` wraps `click.DateTime` so `datetime_type` yields aware
  values. All of its formats either end in a literal `Z` or carry no timezone
  at all, and both mean UTC here.
* `Library.from_api()` normalizes its `start_date` / `end_date` bounds, so
  programmatic callers passing naive values keep working.
* `datetime.utcnow()` and `datetime.utcfromtimestamp()`, deprecated since
  Python 3.12, are replaced by `datetime.now(UTC)` and
  `datetime.fromtimestamp(..., tz=UTC)`.

BREAKING CHANGE: `parse_api_datetime()` and `datetime_type` now return
timezone-aware UTC datetimes. Plugins comparing those results against naive
datetimes will raise `TypeError` and have to be adjusted.
@mkb79
mkb79 force-pushed the refactor/tz-aware-datetimes branch from 37e55ce to 7bafb4c Compare July 29, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant