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

Fix issue with DST starting/ending causing overlaps/gaps #5266

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

zackman0010
Copy link
Contributor

@zackman0010 zackman0010 commented Nov 19, 2024

What this PR does

This older version of recurring_ical_events does not call the pytz .normalize() function, which can cause some invalid datetime objects to return when a DST swap happens. For example: Nov 3, 2024 9:00 AM CDT instead of the correct 8:00 AM CST). By calling tz.normalize on the end date and checking if the time zone information changed, we can detect when DST starts/stops and adjust the end date accordingly.

DST stopping on November 3, 2024 DST starting on March 9, 2025
Before image image
After image image

Which issue(s) this PR closes

Closes #5247

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

@zackman0010 zackman0010 requested a review from a team as a code owner November 19, 2024 03:39
@CLAassistant
Copy link

CLAassistant commented Nov 19, 2024

CLA assistant check
All committers have signed the CLA.

zackman0010 and others added 3 commits November 20, 2024 13:46
By looping through each event and using normalize, the tzinfo might change DST status. We can check for this change in DST status to adjust the end date either forward or backward, depending if DST stopped or started.
Fixing formatting issues as shown by failing lint test on MR
@matiasb matiasb added pr:no public docs Added to a PR that does not require public documentation updates release:patch PR will be added to "Other Changes" section of release notes labels Nov 20, 2024
@matiasb
Copy link
Contributor

matiasb commented Nov 20, 2024

Thanks @zackman0010 for the contribution (reporting the bug, and the fix)!
LGTM 👍 just did a minor refactoring and added a test.

@matiasb matiasb added this pull request to the merge queue Nov 20, 2024
Merged via the queue into grafana:dev with commit 4afba22 Nov 20, 2024
23 of 25 checks passed
@zackman0010 zackman0010 deleted the dst-fix-no-dep-update branch November 20, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates release:patch PR will be added to "Other Changes" section of release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DST starting/ending causes overlap/gap in schedules
3 participants