-
Notifications
You must be signed in to change notification settings - Fork 8
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
Improve Terraform Provider integration testing #1329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm some questions, ping me on mm when responded please!
@@ -37,7 +37,7 @@ services: | |||
JIMM_OAUTH_CLIENT_SECRET: "SwjDofnbDzJDm9iyfUhEp67FfUFMY8L4" | |||
JIMM_OAUTH_SCOPES: "openid profile email" # Space separated list of scopes | |||
JIMM_DASHBOARD_FINAL_REDIRECT_URL: "https://jaas.ai" # Example URL | |||
JIMM_ACCESS_TOKEN_EXPIRY_DURATION: 1h | |||
JIMM_ACCESS_TOKEN_EXPIRY_DURATION: 100h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make local dev less tedious, you don't need to repeatedly login.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah oki I'm with you
@@ -44,7 +44,7 @@ func (d *cacheDialer) Dial(ctx context.Context, ctl *dbmodel.Controller, mt name | |||
return d.dialer.Dial(ctx, ctl, mt, requiredPermissions) | |||
} | |||
rc := d.sfg.DoChan(ctl.Name, func() (interface{}, error) { | |||
return d.dial(ctx, ctl) | |||
return d.dial(ctx, ctl, requiredPermissions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did this work before without the permissions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are default permissions set in the dialer so that probably resolves most issues because we don't add extra permissions in many places, and I believe the cache dialer is not used unless actually running JIMM?
We also probably want to get rid of the cache dialer, but this is still better than before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I need to look over the default permissions, never really fully understood the permission map and what goes where... But oki thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally no need to sign in over and over again, ty
Description
This PR makes some tweaks after testing JIMM with the Juju Terraform Provider. The changes of note are:
GetApplicationOfferConsumeDetails
to exclude the user field which currently causes an unknown error.Partially addresses CSS-6347
Engineering checklist
Check only items that apply