Skip to content

🐛♻️Computational pipelines: Align computation pipeline HTTP codes, fix dynamic cycles bug#8963

Merged
sanderegg merged 21 commits intoITISFoundation:masterfrom
sanderegg:refactor/change-computation-start-http-codes
Mar 27, 2026
Merged

🐛♻️Computational pipelines: Align computation pipeline HTTP codes, fix dynamic cycles bug#8963
sanderegg merged 21 commits intoITISFoundation:masterfrom
sanderegg:refactor/change-computation-start-http-codes

Conversation

@sanderegg
Copy link
Copy Markdown
Member

@sanderegg sanderegg commented Mar 24, 2026

What do these changes do?

  • Refactors computation pipeline start logic to use correct HTTP status codes:
    • 201 when a pipeline is started
    • 200 when pipeline is up-to-date (nothing to run)
    • 409 for cycles or deprecated services
    • 404/503 for specific error cases
  • Fixes bug where cycles among dynamic (interactive) nodes prevented valid computational nodes from running
  • Updates frontend to handle new status codes and improve re-run dialog logic
  • Adds/updates tests to cover dynamic-only cycles and disconnected computational nodes
  • Improves error messages and exception handling for EC2 and configuration errors
  • Updates OpenAPI specs and API-server/webserver mappings for new status codes
  • Refactors internal client interfaces to propagate status codes

⚠️ need to discuss with @bisgaard-itis about the 406 code and its replacement whether it generates issues

Bugfix: it is now possible to run the computational pipeline, even though there is a cycle in the dynamic services

image

Related issue/s

How to test

  • Deploy all backend and frontend services
  • Start a pipeline with only computational nodes: should return 201
  • Re-run the same pipeline: should return 200 and show re-run dialog
  • Create a project with a cycle among dynamic nodes and disconnected computational nodes, then run: only computational nodes should run (201)
  • Break a pipeline with a computational cycle: should return 409
  • Run all tests: pytest in director-v2, webserver, api-server, static-webserver

Dev-ops

@sanderegg sanderegg added this to the Cassata milestone Mar 24, 2026
@sanderegg sanderegg self-assigned this Mar 24, 2026
@sanderegg sanderegg added a:frontend issue affecting the front-end (area group) a:webserver webserver's codebase. Assigning the area is particularly useful for bugs a:director-v2 issue related with the director-v2 service a:apiserver api-server service labels Mar 24, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.48%. Comparing base (ff5ffe4) to head (f3285fe).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8963   +/-   ##
=======================================
  Coverage   87.48%   87.48%           
=======================================
  Files        2068     2068           
  Lines       81581    81586    +5     
  Branches     1434     1434           
=======================================
+ Hits        71368    71375    +7     
+ Misses       9809     9806    -3     
- Partials      404      405    +1     
Flag Coverage Δ
integrationtests 64.13% <92.50%> (+0.08%) ⬆️
unittests 86.36% <88.09%> (-0.01%) ⬇️
Components Coverage Δ
pkg_aws_library 95.29% <ø> (ø)
pkg_celery_library 77.23% <ø> (ø)
pkg_dask_task_models_library 79.37% <ø> (ø)
pkg_models_library 92.55% <ø> (ø)
pkg_notifications_library 84.48% <ø> (ø)
pkg_postgres_database 89.24% <ø> (ø)
pkg_service_integration 72.81% <ø> (ø)
pkg_service_library 70.31% <ø> (ø)
pkg_settings_library 90.48% <ø> (ø)
pkg_simcore_sdk 85.68% <ø> (-0.11%) ⬇️
agent 92.85% <ø> (ø)
api_server 91.37% <100.00%> (-0.01%) ⬇️
autoscaling 95.54% <ø> (ø)
catalog 92.10% <ø> (ø)
clusters_keeper 98.70% <ø> (ø)
dask_sidecar 91.54% <ø> (-0.22%) ⬇️
datcore_adapter 97.95% <ø> (ø)
director 75.53% <ø> (ø)
director_v2 91.71% <100.00%> (+0.05%) ⬆️
dynamic_scheduler 96.02% <ø> (ø)
dynamic_sidecar 88.29% <ø> (ø)
efs_guardian 89.85% <ø> (ø)
invitations 90.93% <ø> (ø)
payments 92.86% <ø> (ø)
resource_usage_tracker 91.43% <ø> (-0.38%) ⬇️
storage 86.73% <ø> (+0.04%) ⬆️
webclient ∅ <ø> (∅)
webserver 86.86% <100.00%> (+0.03%) ⬆️

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 ff5ffe4...f3285fe. 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.

@sanderegg sanderegg force-pushed the refactor/change-computation-start-http-codes branch from e1ab703 to 90f8fae Compare March 24, 2026 17:45
@sanderegg sanderegg requested a review from Copilot March 24, 2026 17:47
Copy link
Copy Markdown
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 aligns computation “start” semantics across director-v2, webserver, api-server, and the frontend by returning clearer HTTP response codes (notably using 200 OK when there is nothing to start vs 201 Created when a run is actually started), and updating clients/tests/specs accordingly.

Changes:

  • Director-v2 computations endpoint now returns 200 when the pipeline is up-to-date / has no runnable computational tasks (instead of treating it as a 422).
  • Webserver director-v2 client plumbing now propagates the downstream status code (200 vs 201) up to the webserver REST endpoint.
  • Frontend + integration/unit tests + OpenAPI specs updated to reflect the new response-code behavior (and related not-found/conflict/service-unavailable mappings).

Reviewed changes

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

Show a summary per file
File Description
services/web/server/tests/integration/01/test_computation.py Updates integration expectations (e.g., restart start → 200) and minor formatting adjustments.
services/web/server/src/simcore_service_webserver/director_v2/_director_v2_service.py Adapts director-v2 request helper call sites to new (payload, status) return shape.
services/web/server/src/simcore_service_webserver/director_v2/_controller/rest.py Propagates downstream 200 vs 201 for computation start to webserver clients.
services/web/server/src/simcore_service_webserver/director_v2/_client_base.py Enhances request helper to accept multiple expected success codes and return (payload, status).
services/web/server/src/simcore_service_webserver/director_v2/_client.py Allows director-v2 “start computation” to succeed with 200 or 201 and returns status upstream.
services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js Handles 200 “up-to-date” start response by prompting to force re-run.
services/director-v2/tests/unit/with_dbs/comp_scheduler/test_api_route_computations.py Updates unit tests to new status codes (e.g., 404/409).
services/director-v2/tests/integration/conftest.py Accepts 200 or 201 from computation creation helper.
services/director-v2/tests/integration/01/test_computation_api.py Updates integration tests to expect 200 when nothing is runnable/up-to-date.
services/director-v2/src/simcore_service_director_v2/modules/db/repositories/comp_tasks/_utils.py Raises a more specific EC2 instance-type not-found error for invalid EC2 type selection.
services/director-v2/src/simcore_service_director_v2/core/errors.py Introduces EC2InstanceTypeNotFoundError and minor message formatting updates.
services/director-v2/src/simcore_service_director_v2/api/routes/computations.py Implements 200 “nothing started” behavior and remaps several error statuses.
services/api-server/src/simcore_service_api_server/services_http/webserver.py Updates exception mapping for computation start to reflect new upstream statuses.
services/api-server/src/simcore_service_api_server/exceptions/backend_errors.py Aligns api-server error status codes (e.g., cluster not found → 404, config error → 503).
api/specs/web-server/_computations.py Updates webserver OpenAPI spec responses for computation start.

@sanderegg sanderegg force-pushed the refactor/change-computation-start-http-codes branch 2 times, most recently from 0f8fb8b to 7f923bb Compare March 26, 2026 08:16
@sanderegg sanderegg requested a review from Copilot March 26, 2026 08:35
Copy link
Copy Markdown
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

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

@sanderegg sanderegg changed the title ♻️Computational REST API: align response codes for more clarity ♻️Computational pipelines: Align computation pipeline HTTP codes, fix dynamic cycles bug Mar 26, 2026
@sanderegg sanderegg changed the title ♻️Computational pipelines: Align computation pipeline HTTP codes, fix dynamic cycles bug 🐛♻️Computational pipelines: Align computation pipeline HTTP codes, fix dynamic cycles bug Mar 26, 2026
@sanderegg sanderegg force-pushed the refactor/change-computation-start-http-codes branch 2 times, most recently from 354c9fe to 8b04211 Compare March 26, 2026 14:33
@sanderegg sanderegg marked this pull request as ready for review March 26, 2026 14:34
Copy link
Copy Markdown
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.

thx

Copy link
Copy Markdown
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Thanks a lot

@sanderegg sanderegg force-pushed the refactor/change-computation-start-http-codes branch from 5bbbfc2 to c381665 Compare March 27, 2026 14:46
@sanderegg sanderegg added the 🤖-automerge marks PR as ready to be merged for Mergify label Mar 27, 2026
@sanderegg
Copy link
Copy Markdown
Member Author

@mergify queue

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 27, 2026

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Required conditions to enter a queue
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule default]:
      • any of [🛡 GitHub branch protection]:
        • check-neutral = deploy to dockerhub
        • check-skipped = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of [🛡 GitHub branch protection]:
        • check-neutral = unit-tests
        • check-skipped = unit-tests
        • check-success = unit-tests
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of [🛡 GitHub branch protection]:
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of [🛡 GitHub branch protection]:
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of [🛡 GitHub branch protection]:
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed

@sonarqubecloud
Copy link
Copy Markdown

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 27, 2026

queue

⚠️ Configuration not compatible with a branch protection setting

Details

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.

@sanderegg sanderegg merged commit 9913368 into ITISFoundation:master Mar 27, 2026
94 checks passed
@sanderegg sanderegg deleted the refactor/change-computation-start-http-codes branch March 27, 2026 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:apiserver api-server service a:director-v2 issue related with the director-v2 service a:frontend issue affecting the front-end (area group) a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

422 error code on sim4life python runner in GUI

8 participants