-
Notifications
You must be signed in to change notification settings - Fork 32
🐛 Monitor Celery tasks cancellation #7514
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
🐛 Monitor Celery tasks cancellation #7514
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7514 +/- ##
==========================================
- Coverage 87.44% 87.27% -0.18%
==========================================
Files 1740 1370 -370
Lines 67389 56951 -10438
Branches 1143 595 -548
==========================================
- Hits 58931 49704 -9227
+ Misses 8138 7061 -1077
+ Partials 320 186 -134
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…rloromeo/osparc-simcore into monitor-celery-tasks-cancellation
matusdrobuliak66
left a comment
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.
Thanks! Can you please add some description to this PR explaining a bit your changes, thanks 👍
…rloromeo/osparc-simcore into monitor-celery-tasks-cancellation
odeimaiz
left a comment
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.
Thanks!
bisgaard-itis
left a comment
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.
Very cool! Thanks a lot 🏊🏻♂️
|
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.
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
services/storage/tests/unit/test_rpc_handlers_simcore_s3.py:616
- Verify that the updated index (2) for accessing the progress value aligns with the new signature of CeleryTaskWorker.set_progress. If the call arguments change in the future, this index may need an update to avoid silent failures in progress reporting.
progress_updates = [x[0][2].actual_value for x in task_progress_spy.call_args_list]
services/storage/tests/unit/test_async_jobs.py:261
- [nitpick] Consider replacing the magic number (60 * 10) with a named constant or adding inline documentation to clarify its purpose, ensuring future maintainers understand this value is chosen to prevent hanging tests.
payload=60 * 10, # test hangs if not cancelled properly
pcrespov
left a comment
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.
thx



What do these changes do?
Add an abort monitor that checks for tasks abortion to cancel the corresponding task.
CeleryWorkers don't write progress in
Celerys task metadata but on separate field of a Redis hash. E.g.:Related issue/s
How to test
Dev-ops checklist