Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add permisisons to crossmodel facade apis #7130
Conversation
| @@ -239,7 +261,7 @@ func makeOfferParamsFromOffer(offer jujucrossmodel.ApplicationOffer) params.Appl | ||
| // FindApplicationOffers gets details about remote applications that match given filter. | ||
| func (api *API) FindApplicationOffers(filters params.OfferFilters) (params.FindApplicationOffersResults, error) { | ||
| var result params.FindApplicationOffersResults | ||
| - offers, err := api.getApplicationOffersDetails(filters) | ||
| + offers, err := api.getApplicationOffersDetails(filters, permission.ReadAccess) |
axw
Mar 21, 2017
Member
seems weird that Find and List have different perms. can you please leave a comment why. I assume it's because the details of offers, only returned by List, are sensitive.
(it also seems weird that List returns details, and Find doesn't...)
wallyworld
Mar 21, 2017
Owner
Add comments. Also, next PR will move Find and Show to a separate controller facade.
| @@ -102,7 +106,17 @@ func (c *offerCommand) Run(_ *cmd.Context) error { | ||
| if err != nil { | ||
| return err | ||
| } | ||
| - return params.ErrorResults{results}.Combine() | ||
| + err = params.ErrorResults{results}.Combine() | ||
| + if err == nil { |
axw
Mar 21, 2017
Member
this feels too clever, can you please just do
if err := params.ErrorResults{results}.Combine(); err != nil {
return err
}that way you're keepin the return err close to where it was checked, and it doesn't risk getting messed up
| + } | ||
| + url := jujucrossmodel.MakeURL(owner.Name(), unqualifiedModelName, c.OfferName, "") | ||
| + ep := strings.Join(c.Endpoints, ", ") | ||
| + ctx.Infof("Application %q endpoints [%s] availble at %q", c.Application, ep, url) |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
wallyworld commentedMar 21, 2017
Please provide the following details to expedite Pull Request review:
Description of change
The crossmodel facade has permission checks added so that:
offer -> admin access
list -> admin access
show -> read access
find -> read access
Also fix a bug in state where an error adding an offer would create part of the offer and fail the next time.
And improve the offer command by printing info after it runs.
QA steps
create CMR offers, list, show, find etc