Skip to content

feat: allow for a configurable time zone database#41

Open
benwilson512 wants to merge 4 commits into
hopsor:mainfrom
CargoSense:main
Open

feat: allow for a configurable time zone database#41
benwilson512 wants to merge 4 commits into
hopsor:mainfrom
CargoSense:main

Conversation

@benwilson512

Copy link
Copy Markdown

Hey folks!

We're in the middle of trying to minimize our dependency on Hackney, after a variety of recently released CVEs and the decision to only remediate them in ~ 4.0. Unfortunately tzdata has a direct dependency on hackney.

That said we were already on a path to remove tzdata as tz has a more rigorous overall approach, and this one library is the standout on our end.

This PR makes tzdata optional, and allows the user to specify one for this library. If one is not specified it falls back to whatever is configured on Elixir itself.

While I realize the motivation for this PR on our end is pretty tangential to this library's goals, I do think the approach of not binding to a specific timezone database is a good one.

@benwilson512 benwilson512 requested a review from hopsor as a code owner June 14, 2026 23:44
@hopsor hopsor changed the title Allow for a configurable time zone database feat: Allow for a configurable time zone database Jun 15, 2026
@hopsor

hopsor commented Jun 15, 2026

Copy link
Copy Markdown
Owner

hi @benwilson512,

First of all, thanks for your contribution. It's great learning that someone is using this package and takes care providing improvements.

I've been thinking about your PR and, taking into account the current maintenance state of tzdata, I was considering directly replacing tzdata with tz. However, as there are multiple choices out there (tz, tzdata, time_zone_info and zoneinfo) that people can be using in their projects, I think it's the best to give them flexibility to use whatever they prefer.

I'm wondering what's the best way of articulating the flexibility you're introducing with this change.

Until now tzdata was a hard dependency and the user of open_hours wasn't required to do anything special. By relying now on Application.get_env(:open_hours, :time_zone_database, Calendar.get_time_zone_database()) we are expecting the users to have set up a timezone database in his application, otherwise open_hours will fail as you can see in the tests. Users having setup a timezone database before hand is the most likely scenario, but not necessarily. So I feel this is a breaking change and we have to properly think about it.

I'm not against introducing breaking changes, actually the opposite. This looks like a good change providing flexibility to the users so it can be a good moment for a first major release, however we need to figure out the best way to do so.

@hopsor hopsor changed the title feat: Allow for a configurable time zone database feat: allow for a configurable time zone database Jun 15, 2026
@benwilson512

benwilson512 commented Jun 15, 2026

Copy link
Copy Markdown
Author

An excellent point. While I think it's in practice extremely unlikely this would mess someone up I do think it's reasonable to make this a 0.4 release. This sort of change would be pretty expected in a pretty 1.0 library.

Notably if for whatever reason a user did not want to configure Elixir itself or if they wanted to configure a different database for open hours, that is possible under this PR.

I will update the PR to ensure the tests pass, as well as provide some changelog / readme entries to cover the full config story.

@hopsor

hopsor commented Jun 15, 2026

Copy link
Copy Markdown
Owner

That'd be awesome @benwilson512

I have just merged a change in the CI that will unblock the elixir 1.18 ci test. It was failing due to a Erlang certificate problem. So please rebase your branch :-)

@benwilson512

Copy link
Copy Markdown
Author

Synced with main and changelog / readme notes added.

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.

2 participants