Skip to content
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

epoch_seconds_to_datetime utility function #132

Merged
merged 2 commits into from
Feb 14, 2023
Merged

epoch_seconds_to_datetime utility function #132

merged 2 commits into from
Feb 14, 2023

Conversation

jonemo
Copy link
Contributor

@jonemo jonemo commented Feb 6, 2023

This addresses an unhandled OverflowError that can occur on 32-bit systems due to the Year 2038 problem or with negative timestamps. I became aware of this from a (currently unresolved) cluster of botocore issue and past attempts at mitigation:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


def test_epoch_seconds_to_datetime_with_overflow_error(monkeypatch):
# Emulate the Year 2038 problem by always raising an OverflowError.
# epoch_seconds_to_datetime() must avoid the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usual outcome of overthinking a code comment while the clock is ticking down to the daily standup meeting...

Comment on lines +14 to +15
# mypy: allow-untyped-defs
# mypy: allow-incomplete-defs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To date, this is the best solution we have for test files that include pytest fixtures.

@@ -1,13 +1,31 @@
# Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To preempt the usual comment: I know, it's 2023. This notice should have been added here in 2022 when the file was first created, so backdating it.

@jonemo jonemo merged commit 4df730c into develop Feb 14, 2023
@nateprewitt nateprewitt deleted the year2038 branch January 16, 2024 19:45
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.

3 participants