Improve python plugin configuration#1605
Conversation
bc02124 to
08c9c75
Compare
sebsura
left a comment
There was a problem hiding this comment.
This is pretty cool, but i do have some questions.
-
Is the idea that each plugin overwrites
config_pathso that each of them has a unique config file ? If not, maybe the core should just load the file into a string and pass that to python so that we do not have to bother with the path stuff inside python. -
If each plugin is supposed to share the same config file, should we use the sections to tell which option belongs to which plugin ? I.e.
[Base] # shared by every plugin
...
[vmware] # only for vmware plugins
...
[my-plugin]
...
This would obviously only work if the plugin knows its name.
- What do you think of using https://docs.python.org/3/library/configparser.html for parsing ? Looks like it has a similar goal and its a standard library (?). That way we can say that we follow a/the standard.
No, that's not the idea.
We're not sharing the configuration files. Retrieving the config-path from core is just so you can configure with relative paths.
Thought about it (especially as it is used in the VMWare plugin), but the mapping code would be more complicated than the "parser" we have for the configuration files now. |
08c9c75 to
3c8832a
Compare
sebsura
left a comment
There was a problem hiding this comment.
I added some additional comments
systemtests/tests/py3plug-fd-basic/python-modules/config-test-module.py
Outdated
Show resolved
Hide resolved
core/src/plugins/filed/python/pyfiles/BareosFdPluginBaseclass.py
Outdated
Show resolved
Hide resolved
core/src/plugins/filed/python/pyfiles/BareosFdPluginBaseclass.py
Outdated
Show resolved
Hide resolved
core/src/plugins/filed/python/pyfiles/BareosFdPluginBaseclass.py
Outdated
Show resolved
Hide resolved
7af197f to
c8728dc
Compare
sebsura
left a comment
There was a problem hiding this comment.
I added some suggestions. I am also not sure if we package the new script on windows at all. Since the windows version does have access to the new plugin option, it should probably also have access to this functionality.
docs/manuals/source/TasksAndConcepts/Plugins/FileDaemonPlugins/PythonFdPlugin.rst.inc
Show resolved
Hide resolved
docs/manuals/source/TasksAndConcepts/Plugins/FileDaemonPlugins/PythonFdPlugin.rst.inc
Outdated
Show resolved
Hide resolved
docs/manuals/source/TasksAndConcepts/Plugins/FileDaemonPlugins/PythonFdPlugin.rst.inc
Outdated
Show resolved
Hide resolved
docs/manuals/source/TasksAndConcepts/Plugins/FileDaemonPlugins/PythonFdPlugin.rst.inc
Outdated
Show resolved
Hide resolved
docs/manuals/source/TasksAndConcepts/Plugins/FileDaemonPlugins/PythonFdPlugin.rst.inc
Outdated
Show resolved
Hide resolved
docs/manuals/source/TasksAndConcepts/Plugins/FileDaemonPlugins/PythonFdPlugin.rst.inc
Outdated
Show resolved
Hide resolved
sebsura
left a comment
There was a problem hiding this comment.
Looks great. I added one small comment; after that is taken care of ill approve this!
This patch adds bVarUsedConfig that provides the path to the configuration that the fd uses. This will contain something like `/etc/bareos/bareos-fd.d/*/*.conf`.
fafc739 to
5a86437
Compare
824877d to
5a86437
Compare
this patch extends BareosFdPluginBaseclass to provide a standard way to work with plugin configuration files. When the plugin options parser encounters one of the options defaults_file or overrides_file, it will treat the values as a path to an ini-style configuration file and will try to read it. The resulting options are added with precedence overrides_file, plugin definition, defaults_file. The code will consume the defaults_file and overrides_file options, so the plugin will not see them later.
by adding a transform (currently only "enc") to an option name, the python baseclass will now decode the value. i.e. if you configure `password#enc=E+*g/GAhM4` you will end up with the option password containing the value "password".
the dictionaries in the module were previously filled with arbitrary numbers that matched what we had in the enums. With this patch we now export the enum value itself, so no mismatches can happen. The patch also converts the filetype defines into an enum.
We now encode with b85encode instead of a85encode. The major difference is the set of characters used to represent the result. While a85encode will use a colon (:) that is also used as an option separator in the plugindef, b85encode will not. As a result values encoded with b85encode will be simpler to use.
5a86437 to
62a3b07
Compare
add support for configuration files
by setting one or both of the options
defaults_fileoroverrides_fileyou can now tell every plugin using the baseclass to read a configuration file.While
defaults_filewill add the values before the plugin definition (i.e. plugin defintion will override the values from that file), usingoverrides_fileswill add the values after defaults and plugin definition (i.e. will override defaults and plugin definition).All paths are also treated relative to the FD's configuration directory to make life easier.
add support for encoded configuration values
if you need to encode a value to make it unreadable (e.g. an api-key), you can now do that. If you - for example - want to set option
passwordto an encoded value, you instead set the optionpassword#encto the encoded value and the baseclass will take care of decoding the value for the plugin.TODO
Please check
If you have any questions or problems, please give a comment in the PR.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
Make sure you check/merge the PR using
devtools/pr-toolto have some simple automated checks run and a proper changelog record added.General
Check backport lineSource code quality
Tests