♻️ Unifying long_running_tasks interfaces #7697

GitHK · 2025-05-19T08:00:55Z

What do these changes do?

Aligned interface between fastapi and aiohttp for long_running_tasks, they no use the same results endpoint. This is a reminder form #3265

Bonus: fixed an issue where dynamic-sidecar did not properly cleanup the directory when the number of files in the input goes from many to one.

Related issue/s

How to test

Dev-ops

codecov · 2025-05-19T08:03:46Z

Codecov Report

Attention: Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 87.12%. Comparing base (c2cf892) to head (4346af9).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7697      +/-   ##
==========================================
+ Coverage   86.80%   87.12%   +0.32%     
==========================================
  Files        1846     1435     -411     
  Lines       71606    59906   -11700     
  Branches     1219      615     -604     
==========================================
- Hits        62158    52194    -9964     
+ Misses       9106     7514    -1592     
+ Partials      342      198     -144

Flag	Coverage Δ
integrationtests	`64.47% <16.66%> (+0.03%)`	⬆️
unittests	`86.19% <75.00%> (-0.37%)`	⬇️

Components	Coverage Δ
api	`∅ <ø> (∅)`
pkg_aws_library	`∅ <ø> (∅)`
pkg_dask_task_models_library	`∅ <ø> (∅)`
pkg_models_library	`∅ <ø> (∅)`
pkg_notifications_library	`∅ <ø> (∅)`
pkg_postgres_database	`∅ <ø> (∅)`
pkg_service_integration	`∅ <ø> (∅)`
pkg_service_library	`71.87% <92.85%> (-0.06%)`	⬇️
pkg_settings_library	`∅ <ø> (∅)`
pkg_simcore_sdk	`85.03% <ø> (-0.06%)`	⬇️
agent	`96.29% <ø> (ø)`
api_server	`91.77% <ø> (ø)`
autoscaling	`96.03% <ø> (ø)`
catalog	`92.25% <ø> (ø)`
clusters_keeper	`99.13% <ø> (ø)`
dask_sidecar	`91.67% <ø> (+0.22%)`	⬆️
datcore_adapter	`97.94% <ø> (ø)`
director	`76.73% <ø> (-0.10%)`	⬇️
director_v2	`91.03% <66.66%> (-0.06%)`	⬇️
dynamic_scheduler	`96.69% <ø> (ø)`
dynamic_sidecar	`90.11% <0.00%> (-0.03%)`	⬇️
efs_guardian	`89.65% <ø> (ø)`
invitations	`93.00% <ø> (ø)`
payments	`92.57% <ø> (ø)`
resource_usage_tracker	`88.98% <ø> (-0.11%)`	⬇️
storage	`87.46% <ø> (-0.36%)`	⬇️
webclient	`∅ <ø> (∅)`
webserver	`85.74% <ø> (+1.78%)`	⬆️

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 c2cf892...4346af9. 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.

…nt-long-running-decorator

Copilot

Pull Request Overview

This PR refactors the long_running_tasks interfaces across fastapi and aiohttp to use a unified exception and result handling approach and removes deprecated APIs. Key changes include:

Changing expected exceptions from TaskClientResultError to RuntimeError or more specific errors.
Removing the deprecated get_task_result_old API and related tests.
Updating import paths and exception handling in both client and server modules.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
services/dynamic-sidecar/tests/unit/test_api_rest_workflow_service_metrics.py	Updated expected exception for failed user services start/stop.
services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py	Changed expected exception to UploadPortsFailedError, aligning with new API.
services/dynamic-sidecar/tests/unit/test_api_rest_containers.py	Removed obsolete error field assertion to reflect updated response body.
services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py	Refactored prunable folder instantiation for cleaner file cleanup.
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py	Removed the catching of TaskClientResultError and now only handling BaseHttpClientError.
packages/service-library/tests/long_running_tasks_task.py	Removed tests for deprecated get_task_result_old functionality.
packages/service-library/tests/fastapi/long_running_tasks_context_manager.py	Updated exception expectations and assertions in context manager tests.
packages/service-library/tests/fastapi/long_running_tasks.py	Refactored tests to use updated exception types and result formats.
packages/service-library/src/servicelib/long_running_tasks/_task.py	Removed deprecated API and adjusted exception catch in async task cancellation.
packages/service-library/src/servicelib/long_running_tasks/_errors.py	Removed TaskClientResultError definition.
packages/service-library/src/servicelib/fastapi/long_running_tasks/server.py	Updated imports to use the new TaskResult model.
packages/service-library/src/servicelib/fastapi/long_running_tasks/client.py	Removed return_exception parameter and TaskClientResultError handling.
packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py	Refactored the get_task_result endpoint to remove legacy behavior.
packages/service-library/src/servicelib/fastapi/long_running_tasks/_context_manager.py	Updated docstring to reflect exception type changes.
packages/service-library/src/servicelib/fastapi/long_running_tasks/_client.py	Simplified get_task_result handling and adjusted import usage.

packages/service-library/src/servicelib/long_running_tasks/_task.py

.../service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py

…sk.py Co-authored-by: Copilot <[email protected]>

sanderegg

thanks! just a few minor comments.

packages/service-library/src/servicelib/fastapi/long_running_tasks/_context_manager.py

.../service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/nodeports.py

bisgaard-itis

Nice cleanup. Thanks

packages/service-library/src/servicelib/fastapi/long_running_tasks/_client.py

packages/service-library/src/servicelib/fastapi/long_running_tasks/_context_manager.py

pcrespov

thx (or ... ty) ;-)

packages/service-library/src/servicelib/fastapi/long_running_tasks/_context_manager.py

packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py

packages/service-library/src/servicelib/long_running_tasks/_errors.py

…nt-long-running-decorator

…om:GitHK/osparc-simcore-forked into pr-osparc-resilient-long-running-decorator

GitHK · 2025-05-27T11:28:05Z

@Mergifyio queue

mergify · 2025-05-27T11:28:11Z

queue

🛑 The pull request has been removed from the queue `default`

The following conditions don't match anymore:

any of: [🔀 queue conditions]
- all of: [📌 queue conditions of queue default]
  - any of: [🛡 GitHub branch protection]
    - check-neutral = system-tests
    - check-skipped = system-tests
    - check-success = system-tests

mergify · 2025-05-27T14:30:20Z

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

GitHK · 2025-05-28T05:08:19Z

@Mergifyio requeue

mergify · 2025-05-28T05:08:51Z

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request.

mergify · 2025-05-28T05:08:52Z

queue

🛑 The pull request has been removed from the queue `default`

The following conditions don't match anymore:

any of: [🔀 queue conditions]
- all of: [📌 queue conditions of queue default]
  - any of: [🛡 GitHub branch protection]
    - check-neutral = system-tests
    - check-skipped = system-tests
    - check-success = system-tests

mergify · 2025-05-28T05:44:09Z

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

GitHK · 2025-05-28T05:46:28Z

@Mergifyio requeue

mergify · 2025-05-28T05:46:59Z

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request.

mergify · 2025-05-28T05:47:00Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 2350f9a

sonarqubecloud · 2025-05-30T05:46:24Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

updating interface

90a1f10

GitHK self-assigned this May 19, 2025

Andrei Neagu added 5 commits May 19, 2025 10:58

fixed broken import

2ddbf36

refactor imports

f2391f1

pylint

f3d7230

fixed broken tests

1ee17a7

Merge remote-tracking branch 'upstream/master' into pr-osparc-resilie…

bca2c54

…nt-long-running-decorator

GitHK added this to the Bazinga! milestone May 19, 2025

Andrei Neagu added 4 commits May 19, 2025 13:33

fixed errors and removed unused

a81a2d8

Merge remote-tracking branch 'upstream/master' into pr-osparc-resilie…

8263a5b

…nt-long-running-decorator

fixed issues with prunning folder

b225e2c

Merge remote-tracking branch 'upstream/master' into pr-osparc-resilie…

9d301f8

…nt-long-running-decorator

GitHK requested a review from Copilot May 21, 2025 10:47

Copilot AI reviewed May 21, 2025

View reviewed changes

packages/service-library/src/servicelib/long_running_tasks/_task.py Outdated Show resolved Hide resolved

.../service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py Outdated Show resolved Hide resolved

Update packages/service-library/src/servicelib/long_running_tasks/_ta…

f1fb325

…sk.py Co-authored-by: Copilot <[email protected]>

GitHK marked this pull request as ready for review May 21, 2025 10:51

GitHK requested review from pcrespov and sanderegg as code owners May 21, 2025 10:51

GitHK requested review from bisgaard-itis, giancarloromeo and matusdrobuliak66 May 21, 2025 10:52

sanderegg approved these changes May 21, 2025

View reviewed changes

bisgaard-itis approved these changes May 22, 2025

View reviewed changes

packages/service-library/src/servicelib/fastapi/long_running_tasks/_client.py Show resolved Hide resolved

packages/service-library/src/servicelib/fastapi/long_running_tasks/_context_manager.py Outdated Show resolved Hide resolved

pcrespov approved these changes May 22, 2025

View reviewed changes

matusdrobuliak66 approved these changes May 26, 2025

View reviewed changes

Andrei Neagu added 4 commits May 27, 2025 11:57

Merge remote-tracking branch 'upstream/master' into pr-osparc-resilie…

72c699d

…nt-long-running-decorator

wording

1cff9e8

new modern error types

53dba5e

refactor test

3d310f4

Merge branch 'pr-osparc-resilient-long-running-decorator' of github.c…

46f2af7

…om:GitHK/osparc-simcore-forked into pr-osparc-resilient-long-running-decorator

GitHK added a:dynamic-sidecar dynamic-sidecar service 🤖-automerge marks PR as ready to be merged for Mergify labels May 27, 2025

Merge branch 'master' into pr-osparc-resilient-long-running-decorator

2c920e3

Merge branch 'master' into pr-osparc-resilient-long-running-decorator

64afae0

Merge branch 'master' into pr-osparc-resilient-long-running-decorator

4346af9

mergify bot merged commit 2350f9a into ITISFoundation:master May 30, 2025
94 of 96 checks passed

GitHK deleted the pr-osparc-resilient-long-running-decorator branch May 30, 2025 10:55

matusdrobuliak66 mentioned this pull request Jun 6, 2025

🚀 Release v1.83.0 #7682

Closed

92 tasks

matusdrobuliak66 mentioned this pull request Aug 5, 2025

🚀 Release v1.84.0 #7947

Closed

88 tasks

♻️ Unifying long_running_tasks interfaces #7697

♻️ Unifying long_running_tasks interfaces #7697

Uh oh!

Conversation

GitHK commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

Related issue/s

How to test

Dev-ops

Uh oh!

codecov bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

sanderegg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bisgaard-itis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pcrespov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GitHK commented May 27, 2025

Uh oh!

mergify bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

mergify bot commented May 27, 2025

The merge conditions cannot be satisfied due to failing checks:

Uh oh!

GitHK commented May 28, 2025

Uh oh!

mergify bot commented May 28, 2025

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request has been removed from the queue default

Uh oh!

mergify bot commented May 28, 2025

The merge conditions cannot be satisfied due to failing checks:

Uh oh!

GitHK commented May 28, 2025

Uh oh!

mergify bot commented May 28, 2025

✅ This pull request will be re-embarked automatically

Uh oh!

mergify bot commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ The pull request has been merged automatically

Uh oh!

sonarqubecloud bot commented May 30, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

GitHK commented May 19, 2025 •

edited

Loading

codecov bot commented May 19, 2025 •

edited

Loading

pcrespov left a comment •

edited

Loading

mergify bot commented May 27, 2025 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented May 28, 2025 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented May 28, 2025 •

edited

Loading