Skip to content
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

feat(github): detecting fatal error for disabling integrations #54286

Merged
merged 20 commits into from
Aug 17, 2023

Conversation

chloeho7
Copy link
Contributor

@chloeho7 chloeho7 commented Aug 7, 2023

Storing fatal error for disabling Github integrations
Milestone 3 of Notify on Disabled Integration project

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 7, 2023
@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #54286 (e0fe056) into master (215fad6) will increase coverage by 25.54%.
The diff coverage is 69.23%.

❗ Current head e0fe056 differs from pull request most recent head 2e7cdba. Consider uploading reports for the commit 2e7cdba to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #54286       +/-   ##
===========================================
+ Coverage   54.33%   79.87%   +25.54%     
===========================================
  Files        5010     5025       +15     
  Lines      213416   213821      +405     
  Branches    36314    36374       +60     
===========================================
+ Hits       115950   170796    +54846     
+ Misses      94501    37786    -56715     
- Partials     2965     5239     +2274     
Files Changed Coverage Δ
src/sentry/conf/server.py 91.66% <ø> (ø)
src/sentry/shared_integrations/client/base.py 76.97% <46.66%> (+52.35%) ⬆️
src/sentry/features/__init__.py 100.00% <100.00%> (ø)
src/sentry/integrations/github/client.py 85.53% <100.00%> (+62.09%) ⬆️
src/sentry/integrations/request_buffer.py 95.23% <100.00%> (+72.61%) ⬆️

... and 1785 files with indirect coverage changes

@chloeho7 chloeho7 marked this pull request as ready for review August 16, 2023 16:18
@chloeho7 chloeho7 requested a review from a team as a code owner August 16, 2023 16:18
) or (
features.has("organizations:github-disable-on-broken", org)
and rpc_integration.provider == "github"
):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be doing a similar check for is_integration_fatal_broken if it is github or is that smth we only want to do for slack?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^ it doesn't seem like we're checking if it's broken before disabling right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is within the disable function which is called after buffer.is_integration_broken() is True. We did is_integration_fatal_broken for slack since we wanted to turn on the slack flag but only validated the fatal broken integrations. I'm not sure if we will have the same situation with github but we can see what we get in the logging and then decide?

@chloeho7 chloeho7 merged commit e97dd74 into master Aug 17, 2023
56 checks passed
@chloeho7 chloeho7 deleted the chloe/github-log-fatal-errors-for-disabling branch August 17, 2023 16:43
@github-actions github-actions bot locked and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants