feat: don't select suspended apps #534
Conversation
This column is part of the model since codecov/shared#237 but was not reflected in the worker yet. related: codecov/internal-issues#519
Currently we ignore if an app is suspended or not. An app is suspended by the user. If we select a suspended app we will fail to get an access_token for it and eventually fail with `InstallationError` These changes filter out suspended apps from being selected. In case all apps for the user are suspended we skip notifications. Other tasks would fail with `NoConfiguredAppsAvailable`. closes codecov/internal-issues#519
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #534 +/- ##
==========================================
+ Coverage 97.48% 97.50% +0.02%
==========================================
Files 418 420 +2
Lines 35013 35396 +383
==========================================
+ Hits 34132 34514 +382
- Misses 881 882 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #534 +/- ##
==========================================
+ Coverage 97.48% 97.50% +0.02%
==========================================
Files 418 420 +2
Lines 35013 35396 +383
==========================================
+ Hits 34132 34514 +382
- Misses 881 882 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #534 +/- ##
==========================================
+ Coverage 97.48% 97.50% +0.02%
==========================================
Files 418 420 +2
Lines 35013 35396 +383
==========================================
+ Hits 34132 34514 +382
- Misses 881 882 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Changes have been made to critical files, which contain lines commonly executed in production. Learn more ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #534 +/- ##
==========================================
+ Coverage 97.50% 97.54% +0.04%
==========================================
Files 449 451 +2
Lines 35736 36561 +825
==========================================
+ Hits 34845 35665 +820
- Misses 891 896 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 20 files with indirect coverage changes This change has been scanned for critical changes. Learn more |
| @@ -225,18 +245,15 @@ def get_github_app_info_for_owner( | |||
| owner, installation_name, repository | |||
| ) | |||
| apps_matching_criteria_count = len(apps_to_consider) | |||
There was a problem hiding this comment.
Does apps_to_consider have a hierarchy of which app to try first over another?
There was a problem hiding this comment.
yes, as documented in the function that produces this list:
worker/services/bots/github_apps.py
Line 58 in 8eb5cc4
The ordering of apps in the list is the hierarchy.
| ) | ||
|
|
||
|
|
||
| def _filter_suspended_apps( |
There was a problem hiding this comment.
Optional: making tests for this that isn't necessarily tethered to the notify test
Currently we ignore if an app is suspended or not.
An app is suspended by the user.
If we select a suspended app we will fail to get an access_token for it
and eventually fail with
InstallationErrorThese changes filter out suspended apps from being selected.
In case all apps for the user are suspended we skip notifications.
Other tasks would fail with
NoConfiguredAppsAvailable.closes https://github.com/codecov/internal-issues/issues/519
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.