code review PR #5
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new `purge_meta_keys` attribue allows to fully manage metadata keys, which means, that unspecified metadata keys are now removed, this attribue is set to true. Comments are now managed in a much better way, which allows adding and removing comments. Still an open issue: when and which comment character should be used? Currently hard coded to '#'
for both: Type kdbkey and provider kdbkey/ruby
implement the first version of kdbmount resource. The kdb provider uses 'kdb' tool to mount and umount files. Currently is very limited. We only support create, destroy and updates to the file path. Changes to plugin settings are not detected.
if prefix is used, otherwise Puppet will display the 'old' unprefixed name in logs only. Can be confusing.
we now can add 'check's to a key, whereas we add them to the spec namespace of the corresponding key. Do a restructure of the test cases. Now we use the very same tests for both kdbkey provider.
autorequire all possible kdbmounts this key can be affected by
on resource create
Allow to manage Elektra array values. Elektra uses the following format for storing array values: .../key = .../key/#0 = first .../key/#1 = second ... When passing an array to the kdbkey value property, update the associated key to reflect this array value (and vice versa).
If an exception occurs only set the error message, do not print any warnings, since the warnings could be misleading because usually warnings are not associated with the manipulated key.
convert single array to string
This ensures, that the check can be performed even if the spec_key is just created (in the same kdb.set operation). Otherwise we could end up creating keys with invalid values
This gives us automatically an order relation ship between parent and child key names (section before key)
Now we carfully check which plugins are currenlty used for mounting the config file and which plugins should really be used.
Kdbmount fixes
handle comments both for 'hosts' and 'ini' style comment metadata.
fix array index names for elements >= 10
markus2330
commented
Jul 5, 2017
| ## Description | ||
|
|
||
| Puppet module for *libelektra* (https://www.libelektra.org). This allows | ||
| key-value based configuration manipulation. |
|
|
||
| * `check`: Add value validation. | ||
|
|
||
| This property allows to define certain restrictions to be applied on the |
| validate do |name| | ||
| unless name.nil? or name.empty? | ||
| unless name =~ /^(\/|spec|proc|dir|user|system)/ | ||
| raise ArgumentError, "'%s' is not a valid basename" % name |
There was a problem hiding this comment.
error is confusing, why basename? do you mean namespace?
It would be better if you get the list of namespaces from the binding (and in the binding from a loop using keyGetNamespace)
|
|
||
| Note: for each 'check/xxx' metadata, required by the Elektra plugins, just | ||
| remove the 'check/' part and add it to the 'check' property here. | ||
| (e.g. validation plugin: 'check/validation' => 'validation' ...) |
There was a problem hiding this comment.
link to doc/METADATA.ini
|
|
||
| ## Release Notes | ||
|
|
||
| Currently the 'libelektra' Puppet module is under heavy development. No releases |
There was a problem hiding this comment.
What about releasing the current version?
|
|
||
| kdbmount { 'user/test/mm': | ||
| file => 'mmtest.json', | ||
| #file => 'mmtest.ini', |
There was a problem hiding this comment.
the name of the file is misleading?
| end | ||
| end | ||
|
|
||
| def flush |
There was a problem hiding this comment.
some code comments would be splendid
| validate do |name| | ||
| if resource[:prefix].nil? or resource[:prefix].empty? | ||
| unless name =~ /^(spec|proc|dir|user|system)?\/.+/ | ||
| raise ArgumentError, "'%s' is not a valid libelektra key name" % name |
| @@ -0,0 +1,15 @@ | |||
| { | |||
| "name": "libelektra-libelektra", | |||
| "version": "0.8.1", | |||
There was a problem hiding this comment.
so it is released after all ;)
| @@ -0,0 +1,15 @@ | |||
| { | |||
| "name": "libelektra-libelektra", | |||
There was a problem hiding this comment.
Is this the correct name?
…_common use require_relative to source the common provider
|
Good to see you work on these issues! But I think you pushed on a wrong branch? |
|
@MasterToney, fixing this might help you getting into the work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is only for a code review, do not merge it.