Release 0.3.0: stop reading the system timezone - #6
Conversation
The default clock was datetime.now(), naive local time, so the date part of a reference depended on the machine that produced it. The same identifier at the same instant became 20260713... on a Lima server and 20260714... on a Tokyo one. Determinism is what this library sells, and it did not survive a fleet. generate_reference() takes tz, defaulting to UTC. generated_at=None reads datetime.now(tz); an aware generated_at is converted into tz; a naive one is taken at face value, as the wall clock the caller chose. Nothing in the module can reach the environment any more, which is the actual fix -- not "use UTC" but "never ask the machine what time it thinks it is". This moves references, and not at the margin: a reference changes whenever the local date differs from the UTC date, which is 21% of the day at UTC-5 and 38% at UTC+9. Stored references are strings in a column and are untouched; what changes is what new calls return. A caller who recomputes rather than stores can pin the old behaviour with the zone their servers ran in. The README and changelog say so plainly, because a caller who finds this out from a lookup miss has already lost. expected_rejected_inserts() lands with it. A unique constraint rejects inserts, not pairs, and the two part company once a bucket fills: two thousand references over three digits is 1999 pairs but 1135 rejections. The pair count also has no ceiling, while rejections cannot exceed the reference count, because an insert can only be rejected once. Callers sizing a retry budget want the latter. Verified against simulation across four orders of magnitude. suffix_length_for() inverts max_references(), which answered the sizing question backwards and left the README instructing callers to write a loop over lengths. Verified to return the smallest length meeting the target, and to round-trip against max_references(). expected_collisions() becomes expected_colliding_pairs(). The old name implied a count of collisions to handle, and until 0.2.1 the docs said so; it counts pairs. The old name warns and goes away in 1.0.0. pytest treats warnings as errors, so it found all 22 call sites that needed migrating.
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
1 participant
The default clock was datetime.now(), naive local time, so the date part of a reference depended on the machine that produced it. The same identifier at the same instant became 20260713... on a Lima server and 20260714... on a Tokyo one. Determinism is what this library sells, and it did not survive a fleet.
generate_reference() takes tz, defaulting to UTC. generated_at=None reads datetime.now(tz); an aware generated_at is converted into tz; a naive one is taken at face value, as the wall clock the caller chose. Nothing in the module can reach the environment any more, which is the actual fix -- not "use UTC" but "never ask the machine what time it thinks it is".
This moves references, and not at the margin: a reference changes whenever the local date differs from the UTC date, which is 21% of the day at UTC-5 and 38% at UTC+9. Stored references are strings in a column and are untouched; what changes is what new calls return. A caller who recomputes rather than stores can pin the old behaviour with the zone their servers ran in. The README and changelog say so plainly, because a caller who finds this out from a lookup miss has already lost.
expected_rejected_inserts() lands with it. A unique constraint rejects inserts, not pairs, and the two part company once a bucket fills: two thousand references over three digits is 1999 pairs but 1135 rejections. The pair count also has no ceiling, while rejections cannot exceed the reference count, because an insert can only be rejected once. Callers sizing a retry budget want the latter. Verified against simulation across four orders of magnitude.
suffix_length_for() inverts max_references(), which answered the sizing question backwards and left the README instructing callers to write a loop over lengths. Verified to return the smallest length meeting the target, and to round-trip against max_references().
expected_collisions() becomes expected_colliding_pairs(). The old name implied a count of collisions to handle, and until 0.2.1 the docs said so; it counts pairs. The old name warns and goes away in 1.0.0. pytest treats warnings as errors, so it found all 22 call sites that needed migrating.