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

kdb set user/ and -N user should behave the same, -f for old behavior #2561

Closed
markus2330 opened this issue Mar 29, 2019 · 10 comments
Closed
Milestone

Comments

@markus2330
Copy link
Contributor

@sanssecours Can you maybe help define the behavior here?

@markus2330 markus2330 self-assigned this Mar 29, 2019
@sanssecours
Copy link
Member

If you mean that something like

kdb get user/tests/get

should behave the same as

kdb get -N user /tests/get

, then I think that sounds reasonable. What exactly is the old behavior? As far as I can tell kdb get does not support an argument -N yet:

kdb 2>&1 get -N
#> kdb: invalid option -- N
#> Sorry, I could not process the given options (see errors above)
#> 
#> Usage: kdb get <name>
#> 
#> Get the value of an individual key.
#> When the key starts with / a cascading lookup will be done.
#> 
#> Example:
#> 
#>    kdb get system/elektra/version/constants/KDB_VERSION
#> 
#> -a --all                 Consider all of the keys.
#> -H --help                Show the man page.
#> -n --no-newline          Suppress the newline at the end of the output.
#> -p --profile <name>      Use a different profile for kdb configuration.
#> -v --verbose             Explain what is happening.
#> -V --version             Print version info.
#> -C --color <when>       Print never/auto(default)/always colored output.

.

@markus2330 markus2330 changed the title kdb get user/ and -N user should behave the same, -f for old behavior kdb set user/ and -N user should behave the same, -f for old behavior Mar 30, 2019
@markus2330
Copy link
Contributor Author

If you mean that something like

exactly, but I meant kdb set

What exactly is the old behavior?

That one way supports the validation and the other does not.

@sanssecours
Copy link
Member

That one way supports the validation and the other does not.

Yes, it make sense that in the following situation:

kdb mount tutorial.dump /tests/spec dump validation
kdb setmeta spec/tests/spec/test check/validation '[1-9][0-9]*'
kdb setmeta spec/tests/spec/test check/validation/match LINE
kdb setmeta spec/tests/spec/test check/validation/message 'Not a number'

, the command

kdb set user/tests/spec/test 'incorrect value'

fails, just as

kdb set -N user /tests/spec/test 'incorrect value'

and

kdb set /tests/spec/test 'incorrect value'

do. Using the option -f (force) to change the value, even if the validation fails sounds like a good choice.

@markus2330
Copy link
Contributor Author

markus2330 commented Mar 31, 2019

Exactly! Thank you for the description of the behavior. Maybe we should also add -N to kdb get so that the same syntax can be used for every kdb command.

@markus2330
Copy link
Contributor Author

Maybe we can fix this together with #2724. Then scripts do not need to be adapted twice.

@kodebach
Copy link
Member

IMO we should always use a cascading parent key. If the given key has a namespace, strip it for creating the parent key, but use it during ksAppendKey (in kdb set) and ksLookup (in kdb get) et al. I think this is the only good solution, since the spec plugin, which is more or less essential for other plugins to work, only works for cascading parent keys.

To disable this behaviour and use the namespace in the parent key as well, we would simply provide a command line option.

@markus2330
Copy link
Contributor Author

IMO we should always use a cascading parent key.

Yes, I agree. It gives a much more consistent user experience.

If the given key has a namespace, [...]

Yes, makes sense. But let us implement this after #3115, if we feel it is needed.

@stale
Copy link

stale bot commented Nov 27, 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 Nov 27, 2020
@stale stale bot removed the stale label Nov 27, 2020
@markus2330
Copy link
Contributor Author

@dev2718 Can you maybe also implement this together with #3742?

dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 12, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 12, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 12, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 17, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 18, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 18, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 18, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 18, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 25, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 25, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 25, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 25, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 29, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 29, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 29, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Aug 29, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Sep 4, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Sep 4, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Sep 4, 2021
dev2718 added a commit to dev2718/libelektra that referenced this issue Sep 4, 2021
@markus2330
Copy link
Contributor Author

Was fixed by @dev2718 🤗

lawli3t pushed a commit to lawli3t/libelektra that referenced this issue Sep 15, 2021
lawli3t pushed a commit to lawli3t/libelektra that referenced this issue Sep 15, 2021
lawli3t pushed a commit to lawli3t/libelektra that referenced this issue Sep 15, 2021
lawli3t pushed a commit to lawli3t/libelektra that referenced this issue Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants