-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Reworking GithubIntegration #2461
Conversation
b42dc61
to
4a402d9
Compare
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2461 +/- ##
==========================================
+ Coverage 98.77% 98.79% +0.01%
==========================================
Files 117 117
Lines 11674 11673 -1
==========================================
+ Hits 11531 11532 +1
+ Misses 143 141 -2
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
can we just have inline type annotation for new file? |
The
GithubIntegration
(Github App installations) can be used as follows:Then,
gh
is agithub.Github
instance, authenticated as Github App installationri
.The arguments of
Github(...)
andGithubIntegration(...)
have to stay in sync, except for those related to authentication. A test is added to assert this.Allows
Requester(jwt=...)
to be given a method instead of a static token string. That method is called whenever a request is made. This ensures the JWT token (which has an expiry) is always up-to-date.A
Requester(app_auth=...)
asksapp_auth._get_access_token_func
to return a function that provides an active token. Method_get_access_token_func
reuses a requester (the calling requester) with JWT authentication. The caching and refreshing is moved intoAppAuthentication
.All requesters use the same values for
verify
,retry
,base_url
, ... as given toGithubIntegration(...)
.Btw., current
AppAuthentication
is better calledAppInstallationAuthentication
, as it authenticates an Github App installation (app id, private key, installation id). The (app id, private key) tuple is used to authentcate as an Github App (e.g. to fetch installations), so that is better calledAppAuthentication
.With this rework, I don't see a need for users to call
create_jwt
orget_access_token
, they will expire anyway.