psyinfra / onyo

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

Design: replace `onyo new --tsv` with `onyo tsv-to-yaml` #722

Closed aqw closed 1 week ago

aqw commented 2 weeks ago

The --tsv option has always felt bolted on to onyo. But the usecase to import CSV/TSV files is absolutely legitimate.

With the development of #703 and #720, I see YAML/keys as the common idiom to use across onyo.

And viewed another way, TSV is just a tabular way to represent YAML.

I propose retiring onyo new --tsv in favor of a dedicate command to build YAML from a TSV file. The resulting output can be redirected into onyo or saved as files and referenced.

Multiple assets should be separated by document separators (---) in the same output.

TobiasKadelka commented 2 weeks ago

Generally, this makes sense to me, except:

Multiple assets should be separated by document separators (---) in the same output.

If you propose to convert a larger tsv table into one yaml file containing multiple documents (not matter if they are already assets or just a yaml file that can be read afterwards), then I would disagree on this, at least until we have better support for this. Because this reads as if we then have a large yaml file which needs to be further converted into multiple actual files to work properly with it in onyo. I think currently most commands can't handle --- correctly?

aqw commented 2 weeks ago

If you propose to convert a larger tsv table into one yaml file containing multiple documents (not matter if they are already assets or just a yaml file that can be read afterwards), then I would disagree on this, at least until we have better support for this. Because this reads as if we then have a large yaml file which needs to be further converted into multiple actual files to work properly with it in onyo. I think currently most commands can't handle --- correctly?

Indeed, I am suggesting a single file with each asset/input separated by ---.

You are correct that this would require that multi-document support be added to #703 and #720.

However, IMO, that is not that complicated. It would follow the exact same rules as --keys (keys must be defined once or N times). Each document is N.

TobiasKadelka commented 2 weeks ago

Then I agree. But I do think "that is not that complicated" is true only for most use-cases, not all of them. We should IMO first discuss how different commands can or can not work on multi-document files before deciding on that specific aspect for the proposed command.

aqw commented 1 week ago

There was broad agreement yesterday. I am closing this design issue in favor of an implementation issue: #729