Repository navigation
docs(telemetry): document the seconds-vs-ms epoch boundary assumption (#121) - #126
Merged
Merged
Conversation
_coerce_timestamp disambiguates seconds from milliseconds purely by magnitude (> 1e12 is treated as ms). This is safe for the data it parses -- a seconds epoch does not reach 1e12 until the year ~33658 and the cloud emits second-resolution TS -- but the assumption was undocumented, so a future reader could mistake the one-sided heuristic for a bug. Add a comment explaining why the threshold is safe and that the converse edge (a pre-2001 ms epoch) is intentionally not defended, and a test pinning each side of the 1e12 boundary (including that a near-boundary seconds value overflows datetime and is dropped rather than mis-parsed). Closes #121
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
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.
Summary
_coerce_timestampdisambiguates seconds from milliseconds purely by magnitude (> 1e12→ divide by 1000). That's safe for the data it parses — a Unix seconds epoch doesn't reach1e12until the year ~33658, and the cloud emits second-resolutionTS— but the assumption was undocumented, so a future reader could mistake the one-sided heuristic for a bug. Closes #121 (low priority, as filed).Per the issue's accepted resolution ("a comment documenting the boundary assumption, or a test pinning the split"), this does both:
1e12is safe for second-epochs, and that the converse edge (a pre-2001 millisecond epoch< 1e12, which the cloud has never produced) is intentionally not defended.1e12, read as seconds, lands in year ~33658 whichdatetimecannot represent, so the existingOverflowError/ValueErrorguard drops it (returnsNone) rather than silently mis-parsing.Type of change
Checklist
ruff check+ruff formatpassmypystrict passes (9 source files)pytestpasses (201); the new test pins the boundary behaviourBehaviour changes worth calling out
None. Comment + test only.