psyinfra / onyo

text-based inventory system on top of git
ISC License
3 stars 5 forks source link

Design: allow `onyo new` to accept values in YAML #703

Open aqw opened 4 weeks ago

aqw commented 4 weeks ago

onyo new can have values passed into it via --template, --keys, or --tsv. However, despite generating a YAML file, there is no way to pass a YAML file into onyo new.

Except one can, by abusing --template. A template does not need to be in .onyo/templates. One could pass a YAML file to --template and proceed from there.

The downside is that one then cannot use a template.

The current two favorite options are: 1) add a flag (such as --yaml) to accept YAML file(s) 2) allow multiple templates to be provided, allowing them to layer

Option 1 is more powerful as a batch import (basically alternative TSV). Option 2 provides interesting new capabilities for templates, to either use them as lego blogs (assembling components) or layers to selectively overwrite or redact values.

These are two very different directions, and I don't think we should pursue both.

Open Questions:

aqw commented 4 weeks ago

I currently favor Option 2. I think bulk import is well covered by TSV or other methods. Giving more power to templates feels like it will have a stronger day-to-day impact, rather than the one-time effort of bulk import.

And perhaps we can be clever with multiple YAML documents in a stream to give templates some multi-asset powers.

TobiasKadelka commented 3 weeks ago

I agree that option 2 makes most sense.

I reflected on the "multiple templates" idea over the weekend. And I began to wonder if for situations when the user enters 2 templates with conflicting information (i.e. in asset 1 key: value1, and in asset 2 key: value2) if it would be possible to do basically what git does for merge conflicts.

As an example, I imagine this call: onyo new --template yaml1 yaml2 --edit opens the editor with:

type: laptop
make: apple
model: mbp
serial: 1234

<<<<
key: value1
====
key: value2
>>>>
aqw commented 3 weeks ago

Oof. I am pretty against the merge conflict syntax. That means we generate a document that is invalid YAML. Loads of issues stem from that. And users who are only casually familiar with git will be confused.

I'd prefer a clear behavior of inheritance and that we just follow those rules.

TobiasKadelka commented 3 weeks ago

I'd prefer a clear behavior of inheritance and that we just follow those rules.

This makes sense to me, but I instantly see some problems. Even if we have some solution like "overwrite keys by order of yaml files given", I am absolutely sure that during the next weeks/months at least somebody in our group will accidently overwrite something while using two files, and we will not notice. I do not think this is really possible to solve with informing people and good documentation. I can see you making the case to go that route, but I wanted to share my perspective.

I wonder then about another possibility: We allow via terminal for onyo new to specify for N assets each key/value either 1 or N times. Maybe the best way to allow multiple yaml without overwriting values would be similar. A template is allowed to specify a value, too (i.e. template and yaml file can both say size: 14 without error), but they are not allowed to be different. Empty fields can be overwritten, but two given pieces of information just are not allowed to overwrite each other.

First I thought this idea blocks use cases and would be stressful, but the more I think about it, the less I can find situations were we would actually use a template for one device, and have another file where we want to overwrite values from it. I think it would most of the time be an unintentional user error.

I know you tend to prefer the more powerful options, but I just think overwriting introductes so many problems that will be hard to identify later.

bpoldrack commented 3 weeks ago

I like option 2. Template layering sounds really good to me.

what to do about empty values? --keys requires both key and value; what if we have a key with a value in the template and the key without a value in the YAML file? Does it retain or overwrite the value with empty?

If we go for option 2, this seems solved. What's specified via --keys takes precedence over the template(s) and templates override eachother in order. The idea of layering would be like class inheritance, I think. You go from the more generic to the more specific. If the more specific one says there's no value, well, then there's no value.

can multiple YAML files be provided?

I think in both - option 1 and 2 - we'd go for allowing the layering. So, yes.

can multiple YAML documents be provided inside of one file? (separated by ---)

I could see that, but I don't see that working with templates. Because it's then ambiguous whether a template with two documents means that the second one is supposed to "update"/override the first one or whether that means that the resulting asset should have two documents. Which is another thing to talk about I suppose. Onyo could only care for the first document in an asset file, but allow more to be in there for now. And have stronger support eventually.

So, the only multi-doc input file I can see right now would be via --yaml where the (total across files?) number of documents needs to match the number of to be created assets. Like the number of same keys one can provide.

I do not quite understand where @TobiasKadelka's thinking goes here, TBH. Templates are overriden. Already. There's no new problem when templates are considered in order. In fact the layering is useful exactly because that's how it would work. Stack templates covering different aspects of assets. Pretty much like deriving classes.

aqw commented 3 weeks ago

Cool. I think we're on very similar pages then. I like taking the layered template route.

However, I do think there's one bit of ambiguity in layering: First template:

key: happy

Second template:

key:

The question is: should key's value could be either happy or empty.

An alternative approach is to expand the vocabulary a bit:

# remove key entirely
key: <unset>
# remove value
key: <empty>
# create the key with an empty value if the key does not yet exist
key: <inherit>

<unset> and <empty> might be a useful differentiation to have for get/set. I don't see any place the <inherit> could be used outside of onyo new. Which might be ok.

This does involve writing those <> values to templates, which makes them a bit different than assets....

aqw commented 3 weeks ago

If there is agreement on #713 , then I think this issue might be ready to be closed in favor of an implementation issue.

I will wait for others to chime in. Let me know and I'll put it together.

aqw commented 2 weeks ago

This is/was /almost/ ready, but then #720 and #722 came along and I want to make sure these all work together.

I find the name --template to be intuitive. I also find the idea of multiple --templates to make sense.

I also think --yaml is direct and unambiguous (much like `--tsv).

I propose:

TobiasKadelka commented 2 weeks ago

That makes a lot of sense to me.

But I think we are now at a point when we need a design issue to discuss which commands can act on multi-document yaml files and which not, and how to act on them, i.e. can onyo set change just the second asset in a multi document file?