Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
audit-log: Walk reply structs to collect errors #8225
Conversation
|
That looks like a spurious failure - retrying here. |
| + } | ||
| + | ||
| + result := make([]*params.Error, value.Len()) | ||
| + for i := 0; i < value.Len(); i++ { |
wallyworld
Dec 14, 2017
Owner
This will leave nil values in the result slice, is that intended?
I guess so because that will result in a nil auditlog.Error value?
babbageclunk
Dec 14, 2017
Member
Yes, that was intentional - although maybe I can make that clearer if by using append instead.
| + item := value.Index(i) | ||
| + // We know item's a struct. | ||
| + errorValue := item.Field(errorField) | ||
| + if errorValue.CanInterface() { |
wallyworld
Dec 14, 2017
Owner
Can we use tryErrorPointer() here?
if errVal, ok := tryErrorPointer(errorValue); ok {
result[i] = errVal
}
or even
result[i], _ = tryErrorPointer(errorValue)
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
jujubot
merged commit 71750c5
into
juju:2.3
Dec 14, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
babbageclunk
deleted the
babbageclunk:audit-errors
branch
Dec 15, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
babbageclunk commentedDec 14, 2017
Description of change
We want to be able to reflect any errors from API methods in the audit log. Since the responses could have any number of errors, we collect them from the response by looking for
*Errorattributes or slices of result objects with*Errorattributes.QA steps
Bootstrap a controller with auditing enabled. Deploy 2 units of ubuntu. Then run
juju remove-unit ubuntu/3 ubuntu/1 ubuntu/2. In the audit log you should see the conversation with theremove-unitcommand, the correspondingApplication.DestroyUnitmethod call, and the error response with:The null in the middle corresponds to the successful removal of ubuntu/1.