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

Use "aware" datetimes #48

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

leandro-lucarella-frequenz
Copy link
Contributor

When creating datetimes we need to make sure to include timezone information, otherwise we will get exceptions raised when comparing to other "aware" datetime objects. Using "native" datetime object (without timezone information) is very dangerous as we might mix datetimes created for different timezones without knowing, introducing very obscure bugs.

More info about "aware" and "native" datetimes: https://docs.python.org/3/library/datetime.html#aware-and-naive-objects

@leandro-lucarella-frequenz leandro-lucarella-frequenz requested a review from a team as a code owner November 16, 2022 12:10
@leandro-lucarella-frequenz leandro-lucarella-frequenz added this to the v0.11.0 milestone Nov 16, 2022
@leandro-lucarella-frequenz leandro-lucarella-frequenz added priority:high Address this as soon as possible type:bug Something isn't working labels Nov 16, 2022
@github-actions github-actions bot added part:channels Affects channels implementation part:tests Affects the unit, integration and performance (benchmarks) tests labels Nov 16, 2022
Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
When creating datetimes we need to make sure to include timezone
information, otherwise we will get exceptions raised when comparing to
other "aware" datetime objects. Using "native" datetime object (without
timezone information) is very dangerous as we might mix datetimes
created for different timezones without knowing, introducing very
obscure bugs.

More info about "aware" and "native" datetimes:
https://docs.python.org/3/library/datetime.html#aware-and-naive-objects

Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
@leandro-lucarella-frequenz
Copy link
Contributor Author

For more context, this is affecting some tests in the SDK, which was really hard to figure out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, that makes sense. Thanks Luca!

@leandro-lucarella-frequenz
Copy link
Contributor Author

It is quite inconvenient (:hankey:) that Python doesn't use "aware" datetimes by default.

@leandro-lucarella-frequenz leandro-lucarella-frequenz merged commit 57731d5 into frequenz-floss:v0.x.x Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:channels Affects channels implementation part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests priority:high Address this as soon as possible type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants