Skip to content

cmd/snap,client: add snap set and snap get commands#1828

Merged
kyrofa merged 5 commits into
canonical:masterfrom
kyrofa:feature/1596629/snap_config_commands
Sep 7, 2016
Merged

cmd/snap,client: add snap set and snap get commands#1828
kyrofa merged 5 commits into
canonical:masterfrom
kyrofa:feature/1596629/snap_config_commands

Conversation

@kyrofa
Copy link
Copy Markdown
Contributor

@kyrofa kyrofa commented Sep 1, 2016

This PR updates LP: #1596629 by adding the snap set and snap get commands and client changes required to communicate with the API. The API changes themselves will be added in a subsequent PR.

This commit adds the `snap set` and `snap get` commands and client
changes required to communicate with the API. The API changes themselves
will be added in a subsequent commit.

Updates: #1596629

Signed-off-by: Kyle Fazzari <kyle@canonical.com>
Comment thread client/config.go Outdated
)

// SetConfig requests a snap to set the provided config.
func (client *Client) SetConfig(snapName string, config map[string]interface{}) (changeID string, err error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this PR made me wonder: since we're shortening the word anyway, can we try to build the feature everywhere around the "conf" term instead of "config"? It's nice, shorter, and reminds of ".conf".

Also, can we please call the parameter above "patch" rather than "conf" or "config", to make it clear that this is not a full config?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright done (I think). Let me know if I missed any!

@niemeyer
Copy link
Copy Markdown
Contributor

niemeyer commented Sep 2, 2016

Thanks for breaking it down. This is looking good. General ideas, and hopefully nothing too controversial.

Signed-off-by: Kyle Fazzari <kyle@canonical.com>
Also update help messages for get and set subcommands.

Signed-off-by: Kyle Fazzari <kyle@canonical.com>
@niemeyer
Copy link
Copy Markdown
Contributor

niemeyer commented Sep 7, 2016

LGTM

Comment thread client/conf.go
if err != nil {
return "", err
}
return client.doAsync("PUT", "/v2/snaps/"+snapName+"/conf", nil, nil, bytes.NewReader(b))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are these endpoints documented?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll document them in the PR that adds them (queued up after this one).

@chipaca
Copy link
Copy Markdown
Contributor

chipaca commented Sep 7, 2016

LGTM, FWIW

@kyrofa kyrofa merged commit 083ce36 into canonical:master Sep 7, 2016
@kyrofa kyrofa deleted the feature/1596629/snap_config_commands branch September 7, 2016 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants