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

fix: Catch full exception instead of runtime error in API usage task #4426

Merged

Conversation

zachaysan
Copy link
Contributor

@zachaysan zachaysan commented Jul 30, 2024

Changes

When there were failures for individual organisations in the handle_api_usage_notifications task the catching code was too restrictive to allow for continued processing after failure. This change simply catches all Exception classes.

How did you test this code?

I ran the test suite but I decided against writing a test for this since the logic is simple.

Edit: It looks like it's failing the code cov check, so I will bite the bullet and write out the test anyway.

@zachaysan zachaysan requested a review from a team as a code owner July 30, 2024 15:17
@zachaysan zachaysan requested review from matthewelwell and removed request for a team July 30, 2024 15:17
Copy link

vercel bot commented Jul 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 5:50pm
flagsmith-frontend-preview ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 5:50pm
flagsmith-frontend-staging ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 5:50pm

@github-actions github-actions bot added the api Issue related to the REST API label Jul 30, 2024
Copy link
Contributor

github-actions bot commented Jul 30, 2024

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-4426 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-4426 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-4426 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-4426 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-4426 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4426 Finished ✅ Results

@zachaysan zachaysan changed the title Fix: Catch full exception instead of runtime error in API usage task fix: Catch full exception instead of runtime error in API usage task Jul 30, 2024
@github-actions github-actions bot added the fix label Jul 30, 2024
Copy link
Contributor

github-actions bot commented Jul 30, 2024

Uffizzi Preview deployment-54810 was deleted.

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.84%. Comparing base (ea5dc09) to head (b26aa6d).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4426   +/-   ##
=======================================
  Coverage   96.84%   96.84%           
=======================================
  Files        1167     1167           
  Lines       38684    38704   +20     
=======================================
+ Hits        37462    37484   +22     
+ Misses       1222     1220    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot added fix and removed fix labels Jul 30, 2024
@github-actions github-actions bot added fix and removed fix labels Jul 30, 2024
@zachaysan zachaysan added this pull request to the merge queue Jul 31, 2024
Merged via the queue into main with commit f03b479 Jul 31, 2024
34 checks passed
@zachaysan zachaysan deleted the fix/catch_full_exception_instead_of_runtime_in_api_usage_task branch July 31, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to the REST API fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants