Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Sep 12, 2025

What do these changes do?

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg added this to the Cheops milestone Sep 12, 2025
@sanderegg sanderegg self-assigned this Sep 12, 2025
@sanderegg sanderegg added the t:maintenance Some planned maintenance work label Sep 12, 2025
@sanderegg sanderegg requested a review from Copilot September 12, 2025 16:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves cancellation error handling in the asyncio codebase by replacing manual task cancellation patterns with the centralized cancel_wait_task utility function. The changes follow best practices for asyncio.CancelledError handling to ensure proper cancellation propagation.

  • Replaces manual task cancellation with cancel_wait_task utility function
  • Improves CancelledError handling to properly propagate cancellations when the owner function is being cancelled
  • Updates redis-commander image and fixes a typo

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
services/web/server/tests/unit/with_dbs/docker-compose-devel.yml Updates redis-commander image and adds user configuration
services/docker-compose-ops.yml Updates redis-commander image and adds user configuration
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_scheduler.py Replaces manual task cancellation with cancel_wait_task utility
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_observer.py Removes unnecessary CancelledError catch-and-reraise pattern
services/catalog/src/simcore_service_catalog/core/background_tasks.py Removes noqa comment from CancelledError handling
packages/service-library/tests/redis/test_decorators.py Fixes typo in comment
packages/service-library/src/servicelib/utils.py Removes ellipsis from function overloads and noqa comment
packages/service-library/src/servicelib/redis/_utils.py Improves type annotations with ParamSpec and TypeVar
packages/service-library/src/servicelib/redis/_decorators.py Replaces manual task cancellation with cancel_wait_task utility
packages/service-library/src/servicelib/long_running_tasks/task.py Adds proper cancellation propagation check
packages/service-library/src/servicelib/fastapi/requests_decorators.py Replaces manual task cancellation with cancel_wait_task utility
packages/service-library/src/servicelib/deferred_tasks/_worker_tracker.py Adds proper cancellation propagation check
packages/service-library/src/servicelib/async_utils.py Replaces manual task cancellation with cancel_wait_task utility
packages/common-library/src/common_library/async_tools.py Improves cancel_wait_task implementation with better error handling
.github/instructions/python.instructions.md Restructures instructions file and adds testing guidelines
.github/instructions/node.instructions.md Creates new Node.js specific instructions file
.github/instructions/general.md.instructions.md Creates general instructions with typo in filename
.github/instructions/general.instructions.md Creates general instructions file

@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 78.37838% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.86%. Comparing base (87820ae) to head (c091814).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8367      +/-   ##
==========================================
- Coverage   87.88%   87.86%   -0.02%     
==========================================
  Files        1945     1945              
  Lines       75620    75626       +6     
  Branches     1316     1321       +5     
==========================================
- Hits        66458    66451       -7     
- Misses       8765     8777      +12     
- Partials      397      398       +1     
Flag Coverage Δ
integrationtests 63.96% <0.00%> (+0.01%) ⬆️
unittests 86.53% <78.37%> (-0.03%) ⬇️
Components Coverage Δ
pkg_aws_library 93.59% <ø> (ø)
pkg_celery_library 84.71% <ø> (ø)
pkg_dask_task_models_library 79.33% <ø> (ø)
pkg_models_library 93.09% <ø> (ø)
pkg_notifications_library 85.20% <ø> (ø)
pkg_postgres_database 87.95% <ø> (ø)
pkg_service_integration 70.19% <ø> (ø)
pkg_service_library 71.73% <75.00%> (-0.22%) ⬇️
pkg_settings_library 90.19% <ø> (ø)
pkg_simcore_sdk 84.97% <ø> (-0.06%) ⬇️
agent 93.53% <ø> (ø)
api_server 91.94% <ø> (ø)
autoscaling 95.77% <ø> (ø)
catalog 92.36% <100.00%> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.37% <ø> (+0.56%) ⬆️
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (ø)
director_v2 90.93% <0.00%> (+0.08%) ⬆️
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.46% <ø> (-0.03%) ⬇️
efs_guardian 89.62% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.61% <ø> (ø)
resource_usage_tracker 91.71% <ø> (-0.54%) ⬇️
storage 86.74% <ø> (-0.09%) ⬇️
webclient ∅ <ø> (∅)
webserver 87.98% <ø> (+0.02%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87820ae...c091814. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2025

🧪 CI Insights

Here's what we observed from your CI run for c091814.

❌ Job Failures

Pipeline Job Health on master Retries 🔍 CI Insights 📄 Logs
PR CI check OAS' are up to date Unknown 0 View View

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI system-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
unit-tests Base branch is healthy, but retries were needed. Could be early signs of flakiness 👀 Healthy 1 View View

@sanderegg sanderegg force-pushed the maintenance/improve-cancellation-error-handling branch from 88e3c72 to 110c727 Compare September 14, 2025 12:51
@sanderegg sanderegg marked this pull request as ready for review September 15, 2025 06:05
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

Great job finding this subtly. Thanks a lot for the explanations. I learned a lot
I left some suggestions
Thx

@sonarqubecloud
Copy link

@sanderegg sanderegg merged commit e0f1ec9 into ITISFoundation:master Sep 15, 2025
143 of 148 checks passed
@sanderegg sanderegg deleted the maintenance/improve-cancellation-error-handling branch September 15, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants