Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

provide a way to enforce the use of the bundled tzdb #228

Closed
BurntSushi opened this issue Jan 29, 2025 · 3 comments
Closed

provide a way to enforce the use of the bundled tzdb #228

BurntSushi opened this issue Jan 29, 2025 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@BurntSushi
Copy link
Owner

I was talking with someone on the Polars team, and they mentioned that they would actually prefer to use a bundled copy of tzdb even if a system zoneinfo was available. This is to ensure that all time zone transition calculations produce the same value across deployments, regardless of the system copy of tzdb.

Jiff can kinda do this today in two ways, but both have costs:

  1. You can enable tzdb-bundle-always and disable tzdb-zoneinfo. But because of feature unification, if another dependency enables tzdb-zoneinfo, you'll wind up using the system copy of the zoneinfo database without meaning to.
  2. You can create a bundled TimeZoneDatabase (the API for this is new and will be part of jiff 0.2) and then use it explicitly. This means not using Timestamp::in_tz or any other convenient parsing functions for Zoned since they all rely the implicit global tzdb. There isn't anything in Jiff that requires the implicit global tzdb, but it makes a lot of APIs much more convenient.

I think (2) is a defensible choice, but I also think it's important to be able to make changes to the global tzdb and its configuration.

I think the most flexible thing to do is to provide a way to set the global tzdb. However, this raises the possibility of some other dependency setting the global tzdb as well. It's kinda like feature unification, but I'd consider it a major faux pas for a library using Jiff to be setting the global tzdb. It should be an application-only kind of thing.

That does kind of make me wonder about the wisdom of being able to set a global tzdb. Today, nobody can set it other than Jiff. That gives one some assurances. So maybe the right solution here is to indeed suggest path (2) above. And I think that while this is more cumbersome than the convenience APIs in Jiff, I think it should be about on par with what you can do with chrono-tz.

@BurntSushi BurntSushi added enhancement New feature or request question Further information is requested labels Jan 29, 2025
@BurntSushi
Copy link
Owner Author

(One of the revelations for me here was that there are use cases where you specifically want to use a bundled tzdb and not the system copy, even if one is available.)

@BurntSushi
Copy link
Owner Author

A point in favor of allowing setting a global tzdb is that crates like log already rely on cooperation since it exposes a way to "set" a global logger. It's a major faux pas for anything but the primary application to set a logger, and I don't think this has been an issue in practice. On the other hand, the tzdb might impact the correctness of an application in a way that logging does not, so perhaps the stakes are higher.

BurntSushi added a commit that referenced this issue Feb 1, 2025
I did this in a previous commit when I refactored `TimeZoneDatabase`,
but updating the CHANGELOG in that commit is too annoying. So just do it
here.

Closes #228
@BurntSushi
Copy link
Owner Author

I think for now I am going to see how far we can get with option (2) in the OP. It's bullet proof in the sense that you won't be at the mercy of feature unification, but you do have to be careful to avoid a few convenience APIs that read from the global tzdb.

I'm still open to adding APIs to "set" the global tzdb, but I'm a little uneasy about doing that and would like some more data on that point.

Option (2) will be possible in jiff 0.2 with the TimeZoneDatabase::bundled() API.

BurntSushi added a commit that referenced this issue Feb 2, 2025
I did this in a previous commit when I refactored `TimeZoneDatabase`,
but updating the CHANGELOG in that commit is too annoying. So just do it
here.

Closes #228
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant