Allow plugins to load specific configurations from Ini file #1357
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1357 +/- ##
==========================================
+ Coverage 87.29% 87.31% +0.02%
==========================================
Files 101 101
Lines 7452 7467 +15
==========================================
+ Hits 6505 6520 +15
Misses 947 947
Continue to review full report at Codecov.
|
|
this does not look like an easy hack to me 😅 |
|
wow, looks cleaner and more scalable now. I can not pretend to fully understand the implementation but if I get it right this would also allow more "dynamic" plugins in the future, e.g. plugins in files that are loaded by configuration settings without needing to adjust the source code of openQA itself. |
With this change we allow the plugins (and not only) to require specific configurations to be loaded.
A Perl module or a Mojolicious Plugin (based on Mojo::Base or Mojolicious::Plugin) if has the method (or attribute, in case of Mojolicious::Plugin) "configuration_fields" and returns a HASH reference, the key/value pairs are cycled to get the relevant configuration from the Ini file parser, and then fill the application config appropriately.
E.g. if we declare a plugin like so:
package OpenQA::MyPlugin::Foo;
use Mojo::Base -base;
has 'configuration_fields' => sub {
{
auth => {
method => 1
}};
};
1;
The server application will fetch the "auth" section and "method" parameter from the Ini parser object, populating the configuration structure inside the application.
See related Progress issue: https://progress.opensuse.org/issues/6192
Delete references to unused auth_config, anyway now is replaced by loading config dinamically by plugins on startup
|
@coolo should we then drop the [easy hack] tag from the poo issue? 😄 @okurz tl;dr it's merely scanning the perl modules loaded into memory for those matching our current namespace (OpenQA::Plugins::* and OpenQA::Auth::*), and if they provides a 'configuration_fields' method (or mojo attribute) gets its content and based on that, fills the application configuration based on the values found on the Ini file parser. But i'm not sure what you meant with "dynamic" plugins, for sure this approach can be extended to other areas, allowing plugin to export/request other parts of the code. |
mudler
changed the title
|
Removed WIP and re-tested it today, seems all green! |
| # because the attributes are not populated until creation. | ||
| my $fields | ||
| = UNIVERSAL::isa($plugin, "Mojo::Base") ? | ||
| do { $plugin->new->configuration_fields() } |
There was a problem hiding this comment.
can you enlighten me on this syntax? looks quite unusual to have a do {} inside a ternary operator. Wouldn't an explicit if/else be easier to read here?
There was a problem hiding this comment.
the do {} it's there just to create a new lexical scope, and to be sure that the on-the-fly plugin instance gets cleaned right after the statement since it's living just on the do {} block - it is not necessary- and it could be also dropped safely. If you prefeer i can move to an if/else equivalent
There was a problem hiding this comment.
nevermind. Let others approve or disapprove :-)
commit da931c4 Merge: f7a233c 8cefc1b Author: Stephan Kulow <coolo@kde.org> AuthorDate: Thu Jun 8 21:21:48 2017 +0200 Commit: GitHub <noreply@github.com> CommitDate: Thu Jun 8 21:21:48 2017 +0200 Merge pull request #1357 from mudler/config_plugin Allow plugins to load specific configurations from Ini file
With this change we allow the plugins (and not only) to require specific configurations to be loaded.
A Perl module or a Mojolicious Plugin (based on Mojo::Base or Mojolicious::Plugin) if has the method (or attribute, in case of Mojolicious::Plugin) "configuration_fields" and returns a HASH reference, the key/value pairs are cycled to get the relevant configuration from the Ini file parser, and then fill the application config instance appropriately.
E.g. if we declare a plugin like so:
The server application will fetch the "auth" section and "method" parameter from the Ini parser object, populating the configuration structure inside the application.
See related Progress issue: https://progress.opensuse.org/issues/6192