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

incus: add completions for storage pools and volumes #597

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

adamcstephens
Copy link
Contributor

Is it ok to bring in code from internal/server ? Or is there another place to get the list of all available storage config options besides

func poolAndVolumeCommonRules(vol *drivers.Volume) map[string]func(string) error {
This could be used for get/unset for pools/volumes

I also couldn't find a list of available storage drivers, but this is only needed for pool create.

Signed-off-by: Adam Stephens <adam@valkor.net>
Signed-off-by: Adam Stephens <adam@valkor.net>
@stgraber
Copy link
Member

stgraber commented Mar 9, 2024

In general you can import from internal in the CLI but importing from internal/server sounds isn't ideal as there's a high chance that this imports a bunch more things and may pull cgo code too.

As we're getting most config keys to be exported to a json file for documentation purposes, I think it will make sense to have the CLI similarly either import the json data at build time or even take it one step further and directly fetch the list of valid config keys through the API.

@adamcstephens
Copy link
Contributor Author

adamcstephens commented Mar 9, 2024

directly fetch the list of valid config keys through the API.

This sounds like a great idea to me. Does this API exist already?

There are a handful of key lists manually maintained in the old bash completion, and I'm doing my best to get parity (and then some) with the old completions. For an individual resource's config keys, I can just pull them from the resource.

Unfortunately, I've only found a nice list of all keys for instances here:

var InstanceConfigKeysAny = map[string]func(value string) error{

It's also possible I'm missing something, and these lists exist for the other resources too. :)

@stgraber
Copy link
Member

stgraber commented Mar 9, 2024

incus query /1.0/metadata/configuration shows you what's currently in there, though I need to clean that stuff up a bit because there's no matching Go client function for it apparently...

We should get a GetMetadataConfiguration() function to grab that data.

@adamcstephens
Copy link
Contributor Author

Sounds good. This should still be ok to merge now, and I can add completions for the missing config keys once #600 is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants