GeezerVison

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 95dcf3e 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 22be349 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

Looks like we don't have tzdata installed on any buildbot / CI 🤔
I don't think it a good idea. Since tzdata does impact the zoneinfo module quite a lot.

CC @pganssle

The ARM Raspbian bot has tzdata installed as a required package, and I suspect most anything else Debian based does as well.

Are we talking about the system tzdata package, or the PyPI package?

I am talking about these lines:

try:
    importlib.metadata.metadata("tzdata")
    HAS_TZDATA_PKG = True
except importlib.metadata.PackageNotFoundError:
    HAS_TZDATA_PKG = False

if HAS_TZDATA_PKG:
    raise ValueError('Fail the test')

That'd be the PyPI package, but that is not required if you have system provided tzdata. I would expect most, if not all, the build bots to have system provided tzdata, so the behavior we are seeing makes sense.

We have two ways of loading tzdata. The first is via tzpath (searches in PYTHONTZPATH and TZDATA), which happens in zoneinfo._tzpath._find_tzfile, with the tzpath being initialized in zoneinfo._tzpath.reset_tzpath. This commonly uses system provided tzdata.

The second one, is via a tzadata package, which happens in zonoinfo._common.load_tzdata, the function name could reflect this. The tzdata package exists to allow the zoneinfo module to still lookup data when the system does not provide tzdata. In these cases, the user can install tzdata via pip install tzdata.

The current tests override tzpath and let ZoneInfo find the tzdata via that mechanism, we don't have a test for load_tzdata it sems, perhaps it would make sense to add it.

Adding the invalid key to test_bad_keys won't do anything because the data will not be loaded via load_tzdata 🤣

The equivalent code for the tzdata codepath is find_tzfile, which does not have the same issue with raising OSError.

Like I mentioned above, I think we need a specific test for load_tzdata.

@sobolevn does that make sense to you? Is there anything you think I am missing? I hope this makes things clearer for you 😅

This PR is stale because it has been open for 30 days with no activity.

This PR is blocked on its author, has not been touched by a human in more than 3 years, and no longer merges cleanly. Closing to cull the number of open pull requests. Do not reopen this pull request without getting a go-ahead from a triager or core developer on gh-96463.