Implement endpoint for setting timezone#145
Draft
duvholt wants to merge 6 commits intochrivers:masterfrom
Draft
Conversation
…configuration at runtime
Owner
|
Very good points! I've bumped into similar problems with the web ui, since we need to be able to rewrite the config, in order to add a backend, for example. Over time, we definitely need to re-think the configuration model. I think it would be much cleaner to configure everything through the web ui, and have some machine-managed configuration along with the state. It would probably be a separate file, but it could even be the same file. In any case, thanks for the interesting PR! I'll take a proper look soon :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm opening this as a draft PR since it depends on config changes that are only in the web ui branch (which I partially cherry picked into this branch).
I replaced tzfile with chrono-tz for two reasons:
Unfortunately this feature won't properly work since timezone is both read from the config (config.yaml) and the state. Some endpoints use the config as a source of truth for this and others are directly from the state. The Hue app uses the resource state so updating the timezone in the app will end up reverting itself after the app refetches data.
Before this PR the timezone in the state file was always based on guessing the timezone.
To properly fix this I think we should introduce some concept of "in memory" state that is read/updated separately from the persistent state. I think all the resources created in
add_bridgefit this definition. This will also likely be needed for the upcoming behavior script I will add in a future PR as otherwise we have to tell users to delete their state file.