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

[#2795] More fine-grained error handling in concurrent case fetching #1421

Conversation

swrichards
Copy link
Collaborator

@swrichards swrichards commented Oct 3, 2024

Taiga 2795.

These changes are necessary because we do not want a failure to resolve a single case to abort the thread for the whole API group. If one case fails, we should still try and return the cases that properly resolve.

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 76.92308% with 6 lines in your changes missing coverage. Please review.

Project coverage is 94.84%. Comparing base (f1745d2) to head (a124447).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/open_inwoner/cms/cases/views/services.py 76.92% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1421      +/-   ##
===========================================
- Coverage    94.85%   94.84%   -0.02%     
===========================================
  Files         1049     1049              
  Lines        38837    38855      +18     
===========================================
+ Hits         36840    36853      +13     
- Misses        1997     2002       +5     

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

@swrichards swrichards marked this pull request as ready for review October 3, 2024 12:09
@swrichards swrichards requested a review from pi-sigma October 3, 2024 12:10
@swrichards swrichards marked this pull request as draft October 3, 2024 12:13
Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

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

Minor suggestion about logging.

These changes are necessary because we do not want a failure to resolve
a single case to abort the thread for the whole API group. If one case
fails, we should still try and return the cases that properly resolve.
@swrichards swrichards force-pushed the issues/2795-better-error-handling-in-concurrent-case-fetching branch from 31ca881 to a124447 Compare October 3, 2024 13:23
@swrichards swrichards requested a review from pi-sigma October 3, 2024 13:25
@swrichards swrichards marked this pull request as ready for review October 3, 2024 13:25
@swrichards
Copy link
Collaborator Author

@pi-sigma thanks for the improvement: I've also raised the timeout limits a bit (I was hitting them locally).

@swrichards swrichards requested a review from alextreme October 3, 2024 14:16
@alextreme alextreme merged commit 715ab6c into develop Oct 4, 2024
20 checks passed
@alextreme alextreme deleted the issues/2795-better-error-handling-in-concurrent-case-fetching branch October 4, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants