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

Add tzdata requirement on Windows #3246

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Conversation

MinchinWeb
Copy link
Contributor

Further to #3161, tzdata is required on Windows, even when using a version of Python that includes zoneinfo in the standard library. This is because Windows doesn't ship with the "standard" timezone database; c.f. PEP 615.

Without this, Pelican 4.9.0 will spit out a series of errors (one for each entry), like:

[10:53:22] ERROR    Could not process .\20121024-push-a-hg-repo-to-github.md                                   log.py:94
                    'No time zone found with key America/Edmonton'

and then finally has a fatal error:

           CRITICAL ZoneInfoNotFoundError: 'No time zone found with key UTC'                             __init__.py:666

I also note that tzdata is listed as a requirement for testing; it may no longer be needed there.

@avaris
Copy link
Member

avaris commented Nov 13, 2023

I also note that tzdata is listed as a requirement for testing; it may no longer be needed there.

I was confused a bit. I was going to say it should show up in tests but that's why tests were passing :). I think it should be removed from test.pip.

Should be installed by pelican directly, if needed, based on OS.
@MinchinWeb
Copy link
Contributor Author

I think it should be removed from test.pip.

done!

@MinchinWeb
Copy link
Contributor Author

FYI, here's where tzdata was added to the testing requirements: 91d9ef7

Add tzdata as dependency in test requirements

Otherwise yields the following error with Python 3.10 on Windows:

zoneinfo._common.ZoneInfoNotFoundError: 'No time zone found with key UTC'

Copy link
Member

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Sorry for this snafu. When I first saw this error, I read something on the Internet that gave me the (clearly-mistaken) impression that the tzdata dependency was something that only needed to be addressed when testing on Windows and that was not otherwise necessary to use Pelican on Windows. Mea culpa.

Many thanks to @MinchinWeb for the fix and to @avaris for reviewing.

@justinmayer justinmayer merged commit a1d475f into getpelican:master Nov 15, 2023
15 checks passed
@MinchinWeb MinchinWeb deleted the tz-windows branch November 17, 2023 04:34
@MinchinWeb
Copy link
Contributor Author

No worries @justinmayer ! You and avaris do a great job of keeping the project moving forward, and make it easy to contribute, and this is still a timely fix. Mostly I just got a little chuckle that I wasn't able to help with your 3.9 release sprint, but instead found this the next day.

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