Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
SLA term required error #7157
Conversation
|
!!test!! |
| + | ||
| +// UserErr returns an error containing a user-friendly message describing how | ||
| +// to agree to required terms. | ||
| +func (e *TermsRequiredError) UserErr() error { |
howbazaar
Mar 27, 2017
Owner
I would like to see some tests for this with one and multiple missing term agreements.
| - terms := strings.Join(err1.Terms, " ") | ||
| - return errors.Errorf(`Declined: please agree to the following terms %s. Try: "juju agree %s"`, terms, terms) | ||
| + if termErr, ok := errors.Cause(err).(*common.TermsRequiredError); ok { | ||
| + return termErr.UserErr() |
| +// MaybeTermsAgreementError returns err as a *TermsAgreementError | ||
| +// if it has a "terms agreement required" error code, otherwise | ||
| +// it returns err unchanged. | ||
| +func MaybeTermsAgreementError(err error) error { |
| + | ||
| +// UserErr returns an error containing a user-friendly message describing how | ||
| +// to agree to required terms. | ||
| +func (e *TermsRequiredError) UserErr() error { |
howbazaar
Mar 27, 2017
Owner
I would like to see some tests for this with one and multiple missing term agreements.
| +func (e *TermsRequiredError) UserErr() error { | ||
| + terms := strings.Join(e.Terms, " ") | ||
| + return errors.Wrap(e, | ||
| + errors.Errorf(`Declined: please agree to the following terms %s. Try: "juju agree %s"`, |
mjs
Mar 27, 2017
Contributor
Do you really need to spell out the terms twice? How about something like this:
Declined: some terms require agreement. Try: "juju agree %s"
?
cmars
Mar 27, 2017
•
Edited 1 time
-
cmars
Mar 27, 2017
Owner
-
Mar 27, 2017
+1, but I'll need to followup on this one as it might break some of our CI tests. I think it'll be fine. Done.
| +) | ||
| + | ||
| +type errorsSuite struct { | ||
| + testing.BaseSuite |
|
$$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 |
cmars commentedMar 24, 2017
Description of change
The current error message displayed when agreement to SLA terms is terrible, something like:
ERROR cannot get discharge from "http://10.232.205.30/terms": third party refused discharge: term agreement required:This change consolidates "term agreement required" error handling so that the error message is handled consistently across several commands including
juju sla.QA steps
juju sla essentialon a new model without having agreed to the required SLA terms would produce a nice error message:Declined: please agree to the following terms term/1. Try: "juju agree term/1"No regressions in this existing error message for other affected commands:
deploy,upgradecharm.Contact @cmars for assistance with setup issues for end-to-end testing.
Documentation changes
Not really. It only changes a new command.
Bug reference
N/A