Test and fix UTC issue with AppInstallationAuth#2561
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #2561 +/- ##
=======================================
Coverage ? 98.35%
=======================================
Files ? 131
Lines ? 13140
Branches ? 0
=======================================
Hits ? 12924
Misses ? 216
Partials ? 0 ☔ View full report in Codecov by Sentry. |
b380f3c to
ca84755
Compare
antoineKorbit
left a comment
There was a problem hiding this comment.
I manually reproduce the code I show in the issue #2560, and it works fine by now. I'm able to do multiple query with the same github rest client as it's shown in the unit test.
Thanks for the quick fix.
|
@antoineKorbit thanks for testing! |
JLLeitschuh
left a comment
There was a problem hiding this comment.
Confused about the proposed change
| ) | ||
| return token_expires_at < datetime.now(timezone.utc) | ||
| # to be fixed by https://github.com/PyGithub/PyGithub/pull/1831 | ||
| return token_expires_at < datetime.now(timezone.utc).replace(tzinfo=None) |
There was a problem hiding this comment.
Sorry, I'm confused. Why are you creating a date with the UTC timezone, then removing the timezone information?
There was a problem hiding this comment.
We need the UTC timestamp here (without timezone information), not the local time (without timezone information). Otherwise under- or overshoot the expiry by the local timezone offset hours.
The token_expires_at is a UTC timestamp without timezone information, so we cannot compare it with a timestamp that has timezone information.
There was a problem hiding this comment.
I understand now, thanks!
ca84755 to
5a700eb
Compare
|
@JLLeitschuh thanks! |
Fixes #2560.