Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Consistently use TZ-aware/unaware datetime values #851

Closed
ranweiler opened this issue May 3, 2021 · 2 comments
Closed

Consistently use TZ-aware/unaware datetime values #851

ranweiler opened this issue May 3, 2021 · 2 comments
Assignees
Labels
bug Something isn't working good first issue

Comments

@ranweiler
Copy link
Member

ranweiler commented May 3, 2021

Python datetime objects can be TZ-aware or unaware. In the service code, we are inconsistent in our usage of one or another (mostly using TZ-unaware).

If we only perform local calculations with objects of the same awareness, no problems can occur. But if they differ and we compare them, they may fail in (inconsistent) ways:

  • == comparisons silently fail, and evaluate False when comparing two semantically-equal datetime objects, but where just one is TZ-aware
  • Inequality checks (<, &c) throw an exception

TZ-awareness appears to not be statically-checkable out of the box. See python/mypy#10067.

We seem to only use TZ-aware code in some parts of the proxy (which caused pain in testing for #839). The quick fix is just to update our usage there. Trying to change the existing (predominantly unaware) code to TZ-aware will likely have a long tail of bugs. A longer-term and safer fix could be to use our own (more statically-constrained) datetime wrappers or subclasses, and move everything to those, to prevent mismatches in the future.

AB#39994539

@ranweiler ranweiler added the bug Something isn't working label May 3, 2021
@ghost ghost added the Needs: triage label May 3, 2021
@glyph
Copy link

glyph commented Jun 12, 2022

You can statically check TZ-awareness using this little library I wrote until Mypy fixes the bug and ideally absorbs it.

@Porges
Copy link
Member

Porges commented Dec 8, 2022

Closing this as service has migrated to C#, where we are using DateTimeOffset consistently.

@Porges Porges closed this as completed Dec 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue
Projects
None yet
Development

No branches or pull requests

5 participants