refactor/improve_config

[NeonJarbas]

replaces #13

[NeonJarbas]

so many tests to rewrite and refactor all the config mocking....

[forslund]

Does this mean that the config files will be read each time a value is fetched from the config?

[forslund]

If that is the case perhaps a file cache can be added and the watchdog module can be leveraged to trigger updates of the cache similar to this: forslund/skill-configuration@b558a86 ?

[JarbasAl]

Does this mean that the config files will be read each time a value is fetched from the config?

this is not the intention, disk reads should only happen when .reload is called explicitly and on first init. The aim is that they are read exactly once unless changed

note that the load stack method only assembles the final merged dict, the configs are still singletons

[JarbasAl]

If that is the case perhaps a file cache can be added and the watchdog module can be leveraged to trigger updates of the cache similar to this: forslund/skill-configuration@b558a86 ?

i like this filewatcher, maybe that can be unified in this new class, that allows TTS/STT to drop their config hash comparison loops

[forslund]

I would probably leave the hash in there since we'd only want to reload if the STT/TTS section of the the config changed. But this is quite useful to make sure changes on disk is reflected in the live config without the need for polling

[JarbasAl]

I would probably leave the hash in there since we'd only want to reload if the STT/TTS section of the the config changed. But this is quite useful to make sure changes on disk is reflected in the live config without the need for polling

the hash itself is not the issue, just mean the loop to check that hash can become event based if we use that file watcher.

I'm thinking about either adding new dedicated events for stt/tts or just listening to the regular configuration.updated and do the hash check on that event

[forslund]

I would probably leave the hash in there since we'd only want to reload if the STT/TTS section of the the config changed. But this is quite useful to make sure changes on disk is reflected in the live config without the need for polling

the hash itself is not the issue, just mean the loop to check that hash can become event based if we use that file watcher.

I'm thinking about either adding new dedicated events for stt/tts or just listening to the regular configuration.updated and do the hash check on that event

Right, that makes a lot of sense. Just make sure to add a lock if you reload the module out-of-line

[JarbasAl]

@forslund i rebased this PR and implemented your FileWatcher class suggestion

I did some refactoring to keep the old class name and increase compatibility, if you feel up to it a new set of eyes would be appreciated :)

for some reason one of the unittests is now failing but i cant figure out why yet.... doesnt seem related to this change, i guess i missed some mock?

[codecov]

Codecov Report

Merging #105 (2a1f180) into dev (6ceb058) will increase coverage by 2.35%.
The diff coverage is 36.85%.

@@            Coverage Diff             @@
##              dev     #105      +/-   ##
==========================================
+ Coverage   50.35%   52.70%   +2.35%     
==========================================
  Files         119      150      +31     
  Lines       10077     9336     -741     
==========================================
- Hits         5074     4921     -153     
+ Misses       5003     4415     -588     

Impacted Files	Coverage Δ
mycroft/audio/__main__.py	0.00% <0.00%> (ø)
mycroft/client/enclosure/__main__.py	0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/arduino.py	0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/eyes.py	0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/mouth.py	0.00% <0.00%> (ø)
mycroft/client/speech/__main__.py	0.00% <0.00%> (ø)
mycroft/client/speech/hotword_factory.py	0.00% <0.00%> (-88.89%)	⬇️
mycroft/client/speech/service.py	0.00% <0.00%> (ø)
mycroft/client/speech/silence.py	0.00% <0.00%> (-42.86%)	⬇️
mycroft/client/text/__init__.py	0.00% <0.00%> (ø)
... and 112 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf7e444...2a1f180. Read the comment docs.

[forslund]

Nice to have the file watcher included!