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

Design: allow keys to reference other keys #714

Open
aqw opened this issue Oct 31, 2024 · 5 comments
Open

Design: allow keys to reference other keys #714

aqw opened this issue Oct 31, 2024 · 5 comments
Labels
design Design decisions in need of discussion

Comments

@aqw
Copy link
Collaborator

aqw commented Oct 31, 2024

It would be useful if values passed to onyo set can refer to other keys in the same asset (i.e. keys should be variables). My initial idea is f-strings.

The initial use-cases that inspired this are:

  1. renaming keys
  2. copying/moving values around (often because a mistake was made)

These two use cases are much more important than they initially appear. We give users a lot of power to get data into onyo. But once the data is in onyo, how easy is it to manipulate the assets? Put another way:

  1. does everything need to be designed perfectly at the beginning, or can my asset definitions evolve?
  2. what if I make a mistake? should I delete everything and start over?

The above use-cases can both already be somewhat met by having onyo get output all of the data, pipe it into awk (for example), and then execute onyo set. However, even that has limitations. Renaming a key that is a dict would be very effortful.

Examples:
Someone has incorrectly assigned inventory numbers to the serial field (this relies on #684):

onyo set --keys inventory.fzj="f'{serial}'" serial=faux --asset warehouse/extdrive_sandisk_*

Rename a key (especially useful if a dict or list):

onyo set --keys new="f'{old}'" old='<unset>' -a warehouse/extdrive_sandisk_*

Open Questions:

  • onyo uses (and should continue to use) {} for dictionaries. f-strings also use {}. Is it possible to clearly differentiate the two?
  • Can the above use-cases be achieved with direct key manipulation? (see Design: onyo list-keys #679 ; and ideas around teaching get and set about keys)
@aqw aqw added the design Design decisions in need of discussion label Oct 31, 2024
@bpoldrack
Copy link
Collaborator

bpoldrack commented Nov 4, 2024

I see two problems with f-strings:

  • users need to learn yet another "language" to express what they want
  • if it's not a f-string-like syntax but used as an actual f-string, it does come with security implications

However, I agree that we need something similar.

  • Our existing expressions for matching and setting are <key-name>=<expr> where the = means to set or match the value of that key. We could have a second "operator" here in order to address the key name itself. A colon for example shouldn't be part of a key name (b/c it has syntactical meaning in YAML). So, <key-name>:<expr> might be used for setting/matching key names instead of their values. This would address the rename-a-key-usecase.

  • As for using keys as variables: We do already have a syntax for that sort of thing (kinda). <unset>, <dict>, etc.
    Maybe anything enclosed in <, > should be an onyo symbol. Hence, a literal <key> could act as a reference to the value of key. Now comes the problem: That would turn unset, dict, list, null, etc. into reserved keys. So, I suggest <:key> as the variable syntax. So, <something> would be an onyo label and <:something> a variable referring to the value of the key something.

With that, your usecases becomes:

Someone has incorrectly assigned inventory numbers to the serial field

onyo set --keys inventory.fzj="<:serial>" serial=faux --asset warehouse/extdrive_sandisk_*

Rename a key (especially useful if a dict or list):

onyo set --keys old:new -a warehouse/extdrive_sandisk_*

This could also allow matching key names (that may be misspelled hence "namespace matching" may not work):

onyo get --match .:.*fzj.* --keys .*fzj.*

which would get all assets that have a key with "fzj" in its name (and print the values of exactly these keys).
Note, that this assumes the key namespace already suggested (. before the : referring to all keys).
One additional implication of that suggestion is of course not only allowing that dot-notation referring to key namespace, but also regex-matching for the --keys option of onyo get, which feels like a natural enhancement to me provided the other things already desired here.

@aqw
Copy link
Collaborator Author

aqw commented Nov 5, 2024

users need to learn yet another "language" to express what they want

I agree. They're already learning regex. At the same time, I can't think of any way to add such a feature without adding a new syntax. So it makes sense to use an existing one.

if it's not a f-string-like syntax but used as an actual f-string, it does come with security implications

That occurred to me, but then I realized: there are no security issues. The user has crafted a query to run on their own copy of the inventory. Anything they exploit/override is exploiting themselves. So rather, they're being powerful/clever rather than exploitative. They are welcome to do insane/awesome things, and we won't stop them.

A colon for example shouldn't be part of a key name (b/c it has syntactical meaning in YAML). So, <key-name>:<expr> might be used for setting/matching key names instead of their values.

Ooooh. Good point. That is interesting. The downside is that we're abusing : to mean something different than what it means in YAML. But this is very interesting...

Rename a key (especially useful if a dict or list):

onyo set --keys old:new -a warehouse/extdrive_sandisk_*

Ahh, so you're imagining onyo set --keys old:new as a shorthand for onyo set --keys new=<:old> old=<unset>

What if we actually do this the other way around. Currently you're prefixing the keyname with :, but I propose suffixing it. For me, it's much more intuitive and reads like YAML.

For example:

onyo set --keys old:new
# becomes
onyo set --keys old:=new

And

onyo get --match '.:.*fzj.*' --keys '.*fzj.*'
# becomes
onyo get --match '.*fzj.*:' --keys '.*fzj.*'

The above is an interesting shorthand, that IMO is valid. It allows avoiding the explicit negation:

onyo get --match '.*fzj.*:(?!<unset>)' --keys '.*fzj.*'

What do you think?

@TobiasKadelka
Copy link
Collaborator

TobiasKadelka commented Nov 5, 2024

users need to learn yet another "language" to express what they want

I agree. They're already learning regex. At the same time, I can't think of any way to add such a feature without > adding a new syntax. So it makes sense to use an existing one.

I generally would agree that adding another language/syntax the user needs to know might be problematic.

However, in this concrete case I am not sure how far that is true because of:

tkadelka in ~/inventory on git:master
❱ cat .onyo/config 

...
[onyo "assets"]
	filename = "{type}_{make}_{model.name}.{serial}"
...

I know this is not technically an f-string (missing the f), but the f-string syntax that addresses keys and is proposed by Alex is literally already part of current onyo -- just in the .onyo/config. It would not require the user to learn something new, it allows to use something already known and very powerful to also work in the terminal, and that for the very real and common use-case of renaming keys.

So from my side a big +1 on the proposed idea of this issue.

@bpoldrack
Copy link
Collaborator

bpoldrack commented Nov 6, 2024

@TobiasKadelka

I know this is not technically an f-string (missing the f), but the f-string syntax that addresses keys and is proposed by Alex is literally already part of current onyo

No. While you said you "know it's not", you don't seem to know. f-strings and python format mini language are not the same thing. Which means: No, it's not the same syntax. It just looks similar (in the simplest cases). Which will confuse the hell out of non-programmers.

@bpoldrack
Copy link
Collaborator

bpoldrack commented Nov 6, 2024

@aqw

Ahh, so you're imagining onyo set --keys old:new as a shorthand for onyo set --keys new=<:old> old=

I would have said "yes", but then I read further and I think you got a different idea by that phrasing, because:

Currently you're prefixing the keyname with :

Nope. The suggested : isn't prefixing or suffixing anything, it's an operation that does the same thing as = except that the target isn't the value of a key but its name. So, the old:new is to be read as "assign the string 'new' to the name of the key that is currently named 'old'".
For simple cases like this example, the difference is academic, but this changes when we look at matching.
We use = for assignment and for matching. In both cases the left hand side says the value of what key we are looking at.
In my proposal, : does the same. It's just the left hand side says the name of what key we are changing or matching against. Which is why I'd find a := a little more weird, because that's clearly an assignment everywhere else.

The colon prefix in variable names is a different thing, that happens to use the same character in my proposal. But really, it's: Something enclosed in < and > is an onyo label, and something enclosed in <: and > is referring to key's value as a variable.

onyo get --match '.:.*fzj.*' --keys '.*fzj.*'
# becomes
onyo get --match '.*fzj.*:' --keys '.*fzj.*'

Hm. My suggestion was using . to refer to all keys (namespaces), then : to say we want to match the key name and then .*fzj.* as a regex to match against key names. So, it follows the same pattern as any other matching expression.
With your "suffix-approach" that's different and I'm not even sure how to read it in order to generalize and see how we parse it. Is your --match '.*fzj.*:' meant to be "fancy key namespace globbing suffixed with a :", or " regex matching against key names suffixed with a :"? How would that translate to, say, "match regex 'A' against key names in the namespace onyo.? For my suggestion, that feels straight-forward to me (assuming .* namespace globbing. If it's going to be just a ., chnage accordingly): --match onyo.*:A, just like doing the same thing but for those keys values would --match onyo.*=A. What would it look like with your proposal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design decisions in need of discussion
Projects
None yet
Development

No branches or pull requests

3 participants