Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spec: behavior of required #1024

Closed
markus2330 opened this issue Oct 15, 2016 · 21 comments
Closed

spec: behavior of required #1024

markus2330 opened this issue Oct 15, 2016 · 21 comments

Comments

@markus2330
Copy link
Contributor

Describe what you wanted to do

I wanted to enforce the presence of some configuration values.
Thus I added required as meta-data of the relevant configuration entries.
For example:

> kdb global-mount # make sure spec is mounted
> kdb setmeta spec/example/key required yes

Describe what you expected

  • That a configuration which does not include the required key gets rejected.
  • That the metadata required is appended to the keys.

Describe what actually happened

> kdb getmeta /example/key required
Metakey not found
> kdb rm /example/key # works, but should not

System Information

  • Elektra Version: master

Further Log Files and Output

@markus2330 markus2330 added this to the 0.8.19 milestone Oct 15, 2016
@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

using spec.ini and spectest.ini from examples/spec gives me:

[test/#2]
 logs/spec/info = #0
 logs/spec/info/#0 = system/testkey/test/#2 has missing subkeys: a
 arrayMeta = array
 conflict/missing/#0 = a
  [testkey/test/#/a]
  require =

if you add conflict/{s,g}et/require ERROR to the specs configuration it should fail with an error

@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

Error (#142) occurred!
Description: globbing error
Ingroup: plugin
Module: spec
At: /home/thomas/Dev/Elektra/libelektra/src/plugins/spec/spec.c:483
Reason: user/testkey/test/#0 has missing subkeys: a

just tested it with ERROR, works for me, but the configuration of spec really is confusing because there are so many options to set

@markus2330
Copy link
Contributor Author

Can you please bring a full example how you tested it with ERROR?

@markus2330
Copy link
Contributor Author

And what about the disappear of required? Meta data required was set in spec/-namespace, and after spec plugin it isn't anymore. Is this a separate issue?

@markus2330
Copy link
Contributor Author

And what is the difference of require and required? Is this a typo in examples/spec/spec.ini?

@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

require means that a key is required, required defines the number of subkeys that are required.
guess we should rename required

by setting system/elektra/globalplugins/postcommit/user/plugins/#0/conflict/get to ERROR spec would yield an error if a key with require metadata is missing

@markus2330
Copy link
Contributor Author

markus2330 commented Oct 16, 2016

require means that a key is required, required defines the number of subkeys that are required.

This was not explained in METADATA.ini, please update.

I would suggest to rename required to require/range or require/number. See also my proposal with array/range in METADATA.ini.

error if a key with require metadata is missing

Does not work for me, I have both (conflict/{get,set} to ERROR (there are range and conflict, subkeys though)), and simplespeclang yields require (see 7936bec) but the validation still does not fail.

@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

 % kdb export system/elektra/globalplugins/
postcommit = list
postcommit/user = list
postcommit/user/placements =
postcommit/user/placements/error = prerollback postrollback
postcommit/user/placements/get = pregetstorage postgetstorage
postcommit/user/placements/set = presetstorage precommit postcommit
postcommit/user/plugins =
postcommit/user/plugins/#0 = spec
postcommit/user/plugins/#0/conflict/get = ERROR
postcommit/user/plugins/#0/placements = spec
postcommit/user/plugins/#0/placements/error =
postcommit/user/plugins/#0/placements/get = postgetstorage
postcommit/user/plugins/#0/placements/set = presetstorage
postgetstorage = list
postrollback = list
precommit = list
pregetstorage = list
prerollback = list
presetstorage = list

 % kdb setmeta /test/a require 1
Using keyname spec/test/a

% kdb set /test test
Using name user/test
Create a new key user/test with string test

 % kdb get /test
test
Error (#142) occurred!
Description: globbing error
Ingroup: plugin
Module: spec
At: /home/thomas/Dev/libelektra/src/plugins/spec/spec.c:483
Reason: user/test has missing subkeys: a

Mountpoint: /test

@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

or per-key config:

 % kdb export system/elektra/globalplugins/postcommit/user/plugins/#0
= spec
placements = spec
placements/error =
placements/get = postgetstorage
placements/set = presetstorage


 % cat /tmp/spec.ini
#@META require = 1
#@META conflict/get/missing = ERROR
[test/a]

 % kdb mount /tmp/spec.ini spec ini
 % kdb set /test test
Using name user/test
Create a new key user/test with string test

 % kdb get /test
test
Error (#142) occurred!
Description: globbing error
Ingroup: plugin
Module: spec
At: /home/thomas/Dev/libelektra/src/plugins/spec/spec.c:483
Reason: user/test has missing subkeys: a

Mountpoint: /test
Configfile: /tmp/spec.ini

@markus2330
Copy link
Contributor Author

For me require and required are removed:

> kdb lsmeta spec/tutorial/schema/notation
check/enum
check/enum/#0
check/enum/#1
check/enum/#2
check/enum/#3
mandatory
require
required
> kdb lsmeta /tutorial/schema/notation
check/enum
check/enum/#0
check/enum/#1
check/enum/#2
check/enum/#3
mandatory

I wonder how it can fail at all, I thought global plugins do not have a way to fail kdb?
I now tried to also set per-key config 0f85a35
If you run the commands of doc/tutorials/validation.md (section "Customized schemas") and then do:

> kdb import -s validate -c "format=% : %" /tutorial/schema simpleini << HERE
notation : graph
applied-to : small
HERE

So import works even though keys are missing:

> kdb ls user/tutorial/schema
user/tutorial/schema/applied-to
user/tutorial/schema/notation

@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

spec removes all plugin-specific metakeys postgetstorage

@tom-wa
Copy link
Contributor

tom-wa commented Oct 16, 2016

% kdb rm -r user/test
Did not find any key

% kdb export spec
#@META conflict/set/missing = ERROR
#@META require = 1
[test/a]

% kdb set /test test
Using name user/test
Create a new key user/test with string test
Error (#142) occurred!
Description: globbing error
Ingroup: plugin
Module: spec
At: /home/thomas/Dev/libelektra/src/plugins/spec/spec.c:483
Reason: user/test has missing subkeys: a

Mountpoint: user
Configfile: /home/thomas/.config/default.ini.15242:1476614312.490320.tmp
 % kdb rm -r user/test
 % kdb export spec
#@META conflict/get/missing = ERROR
#@META require = 1
[test/a]

 % kdb get /test
Did not find key

 % kdb set /test test
Using name user/test
Create a new key user/test with string test

 % kdb get /test
test
Error (#142) occurred!
Description: globbing error
Ingroup: plugin
Module: spec
At: /home/thomas/Dev/libelektra/src/plugins/spec/spec.c:483
Reason: user/test has missing subkeys: a

Mountpoint: /test
Configfile: /tmp/spec.ini

@markus2330
Copy link
Contributor Author

spec removes all plugin-specific metakeys postgetstorage

which is, in this case, not a good idea, require is already set by the spec namespace, so it is not really plugin-specific.

[examples for set]

Yes, I know it works well for small examples. But with multiple keys (where some of that are set) and multiple plugins involved (e.g. enum) there are plenty of issues. It is hard to reproduce something minimalistic but I think if you do whats shown in the tutorial you will find the issues easily.

@tom-wa
Copy link
Contributor

tom-wa commented Nov 17, 2016

ok, found the issue.
if spec/example/_/key has require spec looks for every key matching /example/* and tests for every found key if it has a subkey named key. if no key matching /example/* is found nothing happens

for spec/example/key with require spec looks for the key /example

my intent was for keys like spec/example/#/key to match all the existing members of the array /example/#* and require them to have a subkey key

@markus2330 markus2330 added bug and removed question labels Nov 17, 2016
@markus2330
Copy link
Contributor Author

Is it easy to be fixed?

@tom-wa
Copy link
Contributor

tom-wa commented Nov 21, 2016

not really, it's more like an different approach and works fine for what it does (e.g require every array member to have a specific subkey)
but i'll add a non-wildcard version for the next release too

@markus2330 markus2330 removed this from the 0.8.19 milestone Nov 22, 2016
@markus2330
Copy link
Contributor Author

Can we make "postcommit/user/plugins/#0/conflict/get = ERROR" the default please?

I would like that require works for lcdproc out of the box.

And require seems to be not documented in doc/METADATA.ini.

@kodebach
Copy link
Member

In #2705 (comment) Markus wrote:

we already have #1024 for this. It would be great if you can clean this mess up. I am not sure if we need it for LCDproc, though. I think it would be better to have defaults everywhere.

While it is true that having defaults everywhere is preferrable, LCDproc is actually the perfect example for when require is needed.

A simple example are the displaynames of menus and commands (also the envnames of parameters) in the lcdexec client. In the old version these defaulted to the name of the INI section, which obviously had to be set. Since this is no longer possible, and there simply is no good default for the displayname, it should be required.

Another (even clearer) example, is the ring parameter type. It lets the user select from a set of predefined strings. These strings are given as an array in the config. These strings are per definition required, since we want to provide a selection to the user.

@markus2330
Copy link
Contributor Author

Ok, if require is required, then it is required. Important is that the application will not start up (elektraOpen) not succeed if required keys are missing. Otherwise we lose the guarantees of elektraGet.

@kodebach kodebach moved this from to implement to must have in lcdproc May 26, 2019
@markus2330 markus2330 removed this from must have for release in lcdproc Jul 25, 2019
@stale
Copy link

stale bot commented Jun 19, 2020

I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot added the stale label Jun 19, 2020
@stale
Copy link

stale bot commented Jul 3, 2020

I closed this issue now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot closed this as completed Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants