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

feat: add grants to roles #14

Merged
merged 2 commits into from
Jul 12, 2020
Merged

feat: add grants to roles #14

merged 2 commits into from
Jul 12, 2020

Conversation

malnick
Copy link
Contributor

@malnick malnick commented Jul 10, 2020

No description provided.

return fmt.Errorf("Got an error when reading role %q: %v", id, err)
}

fmt.Printf("[DEBUG] got role for grant: %+v", r)
Copy link
Contributor

Choose a reason for hiding this comment

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

Left over debug log?

if len(grants) > 0 {
r, apiErr, err = r.AddGrants(ctx, grants)
if apiErr != nil || err != nil {
return fmt.Errorf("error setting grants on role:\n API Err: %+v\n Err: %+v\n", *apiErr.Message, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

if apiErr is nil at this point this will result in a panic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update to not reference an attribute of the API error type and include %+v everywhere so we expose the field values in the message. Not perfect, but also won't panic.

@malnick malnick requested a review from talanknight July 11, 2020 03:09
@malnick malnick force-pushed the grants branch 2 times, most recently from c846af3 to 5fc7e5e Compare July 11, 2020 03:16
@malnick malnick merged commit a6b4fa8 into master Jul 12, 2020
@malnick malnick deleted the grants branch July 12, 2020 22:00
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.

2 participants