Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

allow kdbSet even after failed kdbGet #2955

Closed
markus2330 opened this issue Sep 14, 2019 · 7 comments
Closed

allow kdbSet even after failed kdbGet #2955

markus2330 opened this issue Sep 14, 2019 · 7 comments

Comments

@markus2330
Copy link
Contributor

When there is an error in kdbGet() it is impossible to fix the problem using Elektra. There are two seemingly conflicting requirements:

  • Elektra should be able to recover from errors within KDB by using KDB itself.
  • Applications should not continue if kdbGet failed as Elektra then cannot guarantee a valid configuration

I propose that kdbGet will continue to fail (e.g. on syntax errors in config files but also on type errors, where we currently do not fail) but we allow kdbSet to be called (which will overwrite the config in this case). With this feature, we can implement a kdb set -f which will simply continue even if there are errors in kdbGet. Applications do not need such a -f option, they should continue to fail on errors in kdbGet.

Advantages:

  • tooling can even fix severe errors within Elektra (as long as kdbOpen succeeds, everything would be fixable)
  • validation plugins would have the same behavior for get/set (always error if a check fails)
  • we would not need plugin options for plugins to not fail and could greatly simplify the spec plugin

What do you think?

@kodebach
Copy link
Member

There should be one exception: kdb set -f should not work, if a storage plugin failed. If a storage plugin fails, it will return an incomplete or empty KeySet. Therefore kdb set -f /some/key value would delete some or all keys (expect for /some/key of course).

To fix cases where a storage plugin fails, a user would either have to manually fix the storage file, or call kdb rm -r /broken/mountpoint and then kdb set afterwards. Forcing the user to manually call kdb rm, makes it more obvious that data will be lost.

This could be done by splitting the plugin return code ELEKTRA_PLUGIN_STATUS_ERROR into ELEKTRA_PLUGIN_STATUS_ERROR and ELEKTRA_PLUGIN_STATUS_ABORT. The abort return code would indicate that kdbGet() should abort immediately and return an empty KeySet, so that kdbSet() cannot be called.

@markus2330
Copy link
Contributor Author

I like the idea about asking the user to use kdb rm -rf. We should also implement that for mounting (when there were already keys on the mountpoint we have nearly the same situation).

This could be done by splitting the plugin return code ELEKTRA_PLUGIN_STATUS_ERROR into ELEKTRA_PLUGIN_STATUS_ERROR and ELEKTRA_PLUGIN_STATUS_ABORT. The abort return code would indicate that kdbGet() should abort immediately and return an empty KeySet, so that kdbSet() cannot be called.

I do not like the idea to have several channels of communications for errors. We should properly include this in our error concept. Unfortunately, there is currently no way to find out if an error is related to only one key or to several keys. @Piankero do you see a way how to find out such a situation with the error codes?

@kodebach
Copy link
Member

Unfortunately, there is currently no way to find out if an error is related to only one key or to several keys.

We could add a macro that adds a key to the error/cause/key/# meta-array. If the array is empty, we should assume that the whole KeySet is the cause. Then kdb set could proceed with kdbSet() is the key given on the command line is the only one in error/cause/key/#. Similarly kdb import could check that all keys in error/cause/key/# will be overwritten.

Of course this solution requires a lot of changes. However, I don't see a safe solution that doesn't require checking every error in every plugin. Continuing after kdbGet() failed, was never considered possible, so it will always be a lot of work to change that (even in kdbGet() itself).

@ghost
Copy link

ghost commented Sep 16, 2019

@Piankero do you see a way how to find out such a situation with the error codes?

I am not quite sure what the requirements are here exactly for the error codes/system. The discussion here is a bit too deep for my Elektra knowledge.

@markus2330
Copy link
Contributor Author

The suggestion from @kodebach is very good: The idea is to collect the cause of an error (or warning). Usually the cause would be:

  1. either a resource, like the config file. This is already present in error/configfile but would not fit to the error/cause. So we would need to rename error/configfile to error/cause/resource
  2. or (multiple) key(s)

There is of course a very deep relationship to the error codes, so resource errors might have a concrete resource which fails and validation failures should (actually always) have concrete keys as causes.

It would be great if we get a deeper understanding of this.

@stale
Copy link

stale bot commented Sep 16, 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
Copy link

stale bot commented Sep 30, 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 Sep 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants