-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add support for GitHub App authentication #363
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.
Yay! This works and was done really fast! 🚀 I left some comments, questions bellow. Let me know if anything is not clear.
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.
Left couple of comments. The only blocking issue is correctly handling queries when we don't have SelectedAuthType
and default to personal-access-token
. You could either solve it in func New(ctx context.Context, settings models.Settings)
and default to createAccessTokenClient(ctx, settings)
. Or in func LoadSettings(settings backend.DataSourceInstanceSettings)
where you would default to personal-access-token
if no selectedAuthType
is in settings.JSONData
.
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, left a little comment bellow about removing the console.logs.
This PR adds the ability of authenticating with the github data source using GitHub Apps.