Declarative approach to settings

[SKernchen]

• Declarative approach to settings #219
• Adapt Tutorials

[led02]

One minor change.

[zyzzyxdonta]

Similiarly to the JsonConfigSettingsSource defined here, you should be able to write a TomlConfigSettingsSource that reads the config from hermes.toml.

Then you can instantiate the settings like this:

settings = HermesSettings()

at the end of config.py and they will be filled with values from the file Then no manual calls to to model_validate are needed.

(I'm not sure if we can go this route with the plan of plugins being able to configure their own settings.)

[SKernchen]

Similiarly to the JsonConfigSettingsSource defined here, you should be able to write a TomlConfigSettingsSource that reads the config from hermes.toml.

Then you can instantiate the settings like this:

settings = HermesSettings()

at the end of config.py and they will be filled with values from the file Then no manual calls to to model_validate are needed.

(I'm not sure if we can go this route with the plan of plugins being able to configure their own settings.)

I can already skip the model_validate by using the json in the Settings class which uses the HermesSettings. Still defining the Source and automatically reading the Toml could be better for usage.

[zyzzyxdonta]

You call model_validate here. Which makes sense because the settings are read manually, validated once and then accessed using the old code. I.e. hermes.config.get. I meant that this could be avoided.

[led02]

After reviewing the code, some general remarks here:

praise: Worked better than I expected according to your reports. Also your code is well structured and your names seem well chosen. :)

opinion: I think it is not required / advisible to try to keep the exiting code working with declarative model.
In contrast, especially the config.get('foo', 'bar') calls should be avoided.
Instead, there should be a HermesSettings derived class that defines the defaults (where applicable).
While this will bring in lots of minor code breaks, this also brings some major improvements:

• We have a centeral place (for each plugin) where all the required and default values are declared.
• The code to access to the single configuration values is much cleaner.
• We have validation "up front" (i.e., not only when the plugin tries to read the value).
  This is also already given by the validation during configuration but emulation of the dictionaries get behaviour might lead to unforseen effects.

We could discuss whether it would be good to have both options available for transition but I'd prefer to break and repair instead of maintaining deprecated stuff. We could also change your __getattr__ method into a get implementation but this triggers a similar feeling of wrongness deep within my programmers ❤️ ... ;)

hint: I suppose you are using an automatic code formatter? You should definately extend the allowed line length to 120 chars. This will make the code a bit more readable at some places... (This is also the value we are using in our liniting).

[led02]

note: Not working yet, hence no approval.

I added some inline hints and also some general thoughts about the implementation.
I think the way forward here is to implement / adapt all the plugins that access configuration values (i.e., get rid of all the get calls).
Then we can in parallel further refactor the class-based plugin API (i.e., no central settings.py collecting all models anymore) and implement the CLI switch.

Maybe a litte co-programming could help with pushing this forward... ;)

[SKernchen]

Config is not in Context rigth now, maybe @sdruskat attempt for changing to argparse can help there

[led02]

There are still several places where the hermes.logger module is used. However, the config should be accessible using the ctx object (which is already part of the refactoring strategy).