No support for custom configuration file

[m-col]

After posting #113 I realised that visiomode was still trying to read from the default config location. It seems that the default config is really hard-coded in due to conf.Config() in a number of places.

I'm still getting my head around the general structure but it seems a bit non-linear (indeed I've come across a problematic circular import) so a bit of rejigging will be required so that there is One True loaded configuration.

[celefthe]

If you look at visiomode/config.py, there's a default path to a yaml under /etc/visiomode. This is loaded at init time if it exists, with the call to self.load_yaml(path) (which defaults to the above). load_yaml is inherited from mixins.YamlAttributesMixin. If it exists, any attributes matching the ones in Config will be loaded, else if the file doesn't exist nothing happens.

This probably shouldn't fail silently and should instead log, but proper logging is still WIP (issue #34 )

[celefthe]

So when conf.Config is called, it'll try to load from a default config path if it exists. I'm thinking this shouldn't be under /etc/ and should instead sit within the instance directory Flask uses to store session data etc, and the user can interact with it from the "Settings" page in the web panel - I'd rather users don't edit this manually to keep the contents sanitised.

[celefthe]

Right, I read this before I saw #113 .

In retrospect, I think there's no reason to muck about with where the default config path is, so I think hardcoded default and no path option for Config keeps it clean enough. I agree that it's best not to mess about in /etc/, so my thought is - as above - try to read a config yaml from the instance directory if it exists and have the "one true way" to do configure the application be via the Settings page.

I see passing around a config_path as more trouble than it's worth, since the instance directory is used by the app to store data for the sessions already, and gets set up in a defined location depending on how / where the app is installed. I'm open to ideas though

[m-col]

My only concern about using a settings page is the trouble it takes to configure multiple machines. I touched upon that in the PR, where I mentioned that a single config yaml could be read by multiple machines. Obviously I've not actually used the software so perhaps this is not a worry. Thoughts?

As a side-note, load_yaml does not work (at least for config.Config) as object.__dict__ does not include class attributes.

[celefthe]

Yep ok I see your point -- I agree, it's important if you're scaling up and you just want to copy-paste things over and not muck about with the web panel again.

I'll open a separate issue for load_yaml, I wrote that thing ages ago

[celefthe]

Just re-reading this -- did you mean multiple machines reading the same config file from a remote path?

[m-col]

I did yeah

[celefthe]

Ahhh yeah ok now I getcha.

Not sure that'll be too common a use case, or one that matters too much. I think the simplicity of importing conf.Config() wherever you need it, vs having to pass the path around all the time, trumps the convenience of running everything off a single remove config file instead of just copy/pasting (or using the webpanel to import/export). It's a power user thing to do that, and at that stage we can assume if that sort of user is such inclined they'll be able to use symlinks to do it.

Importing conf.Config might be important for plugins (as it is for e.g. stimuli), and passing a custom path to them just in case makes it a bit convoluted in my mind.

[github-actions]

Stale issue message

[github-actions]

Stale issue message

[github-actions]

Stale issue message