Update Installation object with attributes and related methods #1791
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1791 +/- ##
==========================================
+ Coverage 98.76% 98.86% +0.09%
==========================================
Files 51 50 -1
Lines 2677 2636 -41
==========================================
- Hits 2644 2606 -38
+ Misses 33 30 -3
Continue to review full report at Codecov.
|
| """ | ||
| return self._updated_at.value | ||
|
|
||
| def get_repos(self): |
There was a problem hiding this comment.
I also don't think this method belongs in the Installation object as that is a NonCompletable and also the API call does not depend on any of the Installation attributes. It should be moved to the github.Github.get_installation_repos.
There was a problem hiding this comment.
That would be nice, particularly if you only have an installation token and are not generating one. This is what happens in Github Actions.
There was a problem hiding this comment.
Class github.Github is already cluttered with lots of class specific methods.
Why is Github.get_installation_repos better than installation.get_repos? With the latter it is clear that this requires an installation auth, while Github can be used with lots of different authentications, but that method works only with an installation auth.
|
@s-t-e-v-e-n-k Hey, can you take a look at this? Thanks. |
s-t-e-v-e-n-k
left a comment
There was a problem hiding this comment.
My biggest concern with this is it is breaks all users of Installation since it pretty much rewrites it. Can you see any sort of migration plan here?
Codecov Report
@@ Coverage Diff @@
## master #1791 +/- ##
=======================================
Coverage 98.85% 98.86%
=======================================
Files 108 108
Lines 11064 11141 +77
=======================================
+ Hits 10937 11014 +77
Misses 127 127
Continue to review full report at Codecov.
|
579aa3c to
407856a
Compare
I will restore the method in |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Any updates here? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Is this still relevant or should I close this? |
|
I'd like to see this feature implemented as well. I was just in the process of automating the monthly permissions check on our org and was surprised to see that I cannot really query installed apps properly. |
| headers, data = self.__requester.requestJsonAndCheck("GET", f"/apps/{slug}") | ||
| return GithubApp.GithubApp(self.__requester, headers, data, completed=True) | ||
|
|
||
| def get_installation_by_id(self, installation_id): |
There was a problem hiding this comment.
This is now implemented by GithubInstallation.get_app_installation(installation_id).
| self.__requester, headers, data, completed=True | ||
| ) | ||
|
|
||
| def get_installation_by_repo(self, owner, repo): |
There was a problem hiding this comment.
This is now implemented by GithubIntegration.get_repo_installation(owner, repo).
| """ | ||
| return self._updated_at.value | ||
|
|
||
| def get_repos(self): |
There was a problem hiding this comment.
Class github.Github is already cluttered with lots of class specific methods.
Why is Github.get_installation_repos better than installation.get_repos? With the latter it is clear that this requires an installation auth, while Github can be used with lots of different authentications, but that method works only with an installation auth.
|
Honestly, I've actually lost context regarding this. I might need to look at it from scratch to pick it up but I don't want to spend time if it has become irrelevant. If anyone else wants to continue the work done here, please feel free to do so. I'm happy to help :) |
get_installation_by_id()method in the Main class (tested locally)GithubIntegration.get_installation()toGithub.get_installation_by_repo()(tested locally)Github.get_installation_by_id()Github.get_installation_by_repo()Also, the
Installation.get_repos()method should be moved to the main class asGithub.get_installation_repos()because that method requires an installation access token and also theInstallationobject isNonCompletableGithubObject.Fixes: #1781
Fixes: #1776