-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Fixes #13331] Celery-related error handling using structured error data instead of exceptions during harvesting #13332
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13332 +/- ##
==========================================
+ Coverage 72.92% 72.97% +0.05%
==========================================
Files 916 917 +1
Lines 52373 52539 +166
Branches 6001 6005 +4
==========================================
+ Hits 38193 38341 +148
- Misses 12622 12639 +17
- Partials 1558 1559 +1 🚀 New features to boost your workflow:
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The code changes introduce a different approach in error handling in the harvesting app. Instead of relying solely on the on_error method in the finalizer, the code handles errors directly within the _harvest_resource task and returns structured error information. The review includes suggestions to improve the robustness of the error handling logic and enhance code clarity.
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.4.x 4.4.x
# Navigate to the new working tree
cd .worktrees/backport-4.4.x
# Create a new branch
git switch --create backport-13332-to-4.4.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ebacb5cd18857f7eb0f6b8ef0d171e4729ab142f
# Push it to GitHub
git push --set-upstream origin backport-13332-to-4.4.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.4.x Then, create a pull request where the |
In this PR, a different approach in the error handling is proposed in the harvesting app (tasks.py). Rather than relying solely on the on_error method in the finalizer, we could handle errors directly within the _harvest_resource task and return structured error information, for example:
in case of an error, or otherwise:
With this approach, no exceptions are raised from Celery’s perspective. Instead, we return informative result data, which the finalizer can then evaluate to determine overall success or failure. This method results in clearer, more predictable error handling and improves transparency in the harvesting process. Using this method the on_error method is not necessary but we can leave it as it is for extra safety.
Related issue: #13331
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.