search

kbialek / deye-inverter-mqtt

Reads Deye solar inverter metrics and posts them over MQTT
Apache License 2.0
230 stars 50 forks source link

New setting to explicitly enable plugins #184

Closed hoegaarden closed 4 months ago

hoegaarden commented 5 months ago

This introduces a new config value PLUGINS_ENABLED, which is a list of plugins which should be loaded after being discovered successfully.

Use-case:

A container image can be produced, shipping multiple plugins, but still giving users control over which ones get actually loaded.

kbialek commented 5 months ago

hi @hoegaarden, Why opt-out instead of opt-in? Or in other words why PLUGINS_DISABLE and not PLUGINS_ENABLE?

hoegaarden commented 5 months ago

Don't have a strong opinion on that.

To me it makes slightly more sense to, by default, load all plugins that happen to reside in the plugins directory, but allow users to selectively disable certain plugins. FWIW, tho probably not super important: this keeps the current default behavior.

But again, no strong opinion.

kbialek commented 5 months ago

I prefer to focus on and specify what I need instead of what I do not need. Also, imagine what will happen when you release a new version of your image, with some new plugins bundled. They will be automatically activated. I doubt if that is desired.

EDIT: My suggestion is to implement PLUGINS_ENABLED and break backwards compatibility here. I will highlight it in the release notes then.

hoegaarden commented 5 months ago

/ptal @kbialek