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

Restore ability to use updated settings without restarting mkchromecast #454

Open
xsdg opened this issue Feb 14, 2024 · 1 comment
Open
Assignees

Comments

@xsdg
Copy link
Collaborator

xsdg commented Feb 14, 2024

The Config reimplementation in #453 breaks the ability to update settings and then have those updated settings be used without restarting the program.

Here are a few approaches of different complexities to fix this:

  1. Have __init__.py/Mkchromecast class always reread the config file when config-backed attributes are read
  2. Same as # 1, but check mtime before re-parsing (so stat on every access, and only re-parse when the file has been updated)
  3. Same as # 1, but require live config updates (which only originate from preferences.py) to trigger an update in Mkchromecast. This would require some kind of in-memory mtime or update counter as a class member, since the Mkchromecast instances are otherwise independent and may have been instantiated prior to the live config update
  4. Update things so that Mkchromecast is either a true singleton, or is instantiated once and passed around to all the dependencies, instead of having them create instances on their own.
@xsdg xsdg self-assigned this Feb 14, 2024
@xsdg
Copy link
Collaborator Author

xsdg commented May 20, 2024

Adding a bit more context here, since it's related to the long-term fix for #464

How this used to work is that various components around the app (including from the tray) would use preferences.ConfigSectionMap and config.config_manager, or a bare RawConfigParser (from python stdlib) to read and update preferences in close to realtime. This meant that updates would be saved to the mkchromecast config file, and then each time some code wanted to use a config value, it would generally (but not necessarily always) force its config_manager instance to re-read the on-disk config file before reading those values, which would ensure that the values were up-to-date.

The downside is that the config file would be re-parsed and re-read way more times than actually necessary (especially since we know that all configuration changes while mkchromecast is running would originate from preferences.py). Also, because this was being done independently all over the place, there was no real guarantee that every usage point was doing the same thing and would offer the same consistency guarantees.

The update in #453 got rid of ConfigSectionMap as well as all usage of the bare RawConfigParser, and encapsulated all configuration reading/updating within a new config.Config class. And because the codebase has standardized on reading settings from the __init__.Mkchromecast class, it makes sense to have Mkchromecast manage config options coming from the config file versus from cmdline arguments depending on whether we're in Tray mode, and having some kind of manually-triggered updating behavior when preferences.py causes a config update.

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

No branches or pull requests

1 participant