Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Sep 23, 2025

What do these changes do?

Related issue/s

Added placeholder UI which is displayed when DYNAMIC_SCHEDULER_USE_INTERNAL_SCHEDULER is True.
Refactored and fixed tests to accomodate both UIs. The NiceGUI application was not being reset properly between tests.

How to test

Dev-ops

@GitHK GitHK self-assigned this Sep 23, 2025
@GitHK GitHK added this to the Cheops milestone Sep 23, 2025
@GitHK GitHK requested a review from Copilot September 23, 2025 15:26
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 adds a placeholder UI for the internal scheduler in the dynamic-scheduler service. It introduces conditional rendering based on the DYNAMIC_SCHEDULER_USE_INTERNAL_SCHEDULER setting to display either the external or internal scheduler UI.

  • Added new internal scheduler routes with a placeholder UI page
  • Refactored test configuration to properly reset NiceGUI app state between tests
  • Fixed model schema access to use the correct Pydantic v2 method

Reviewed Changes

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

Show a summary per file
File Description
src/simcore_service_dynamic_scheduler/api/frontend/routes_internal_scheduler/__init__.py Creates router module for internal scheduler routes
src/simcore_service_dynamic_scheduler/api/frontend/routes_internal_scheduler/_index.py Implements placeholder UI page for internal scheduler
src/simcore_service_dynamic_scheduler/api/frontend/_setup.py Adds conditional router inclusion based on scheduler type setting
src/simcore_service_dynamic_scheduler/api/frontend/routes_external_scheduler/_service.py Fixes import order for rabbitmq module
tests/unit/api_frontend/conftest.py Refactors test setup with proper NiceGUI reset and scheduler configuration
tests/unit/api_frontend/helpers.py Updates model schema access to use Pydantic v2 syntax
tests/unit/api_frontend/_routes_internal_scheduler/conftest.py Configures tests to use internal scheduler
tests/unit/api_frontend/_routes_internal_scheduler/test__index_.py Tests the internal scheduler placeholder UI
tests/unit/api_frontend/routes_external_scheduler/conftest.py Configures tests to use external scheduler with service mocks

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@GitHK GitHK marked this pull request as ready for review September 23, 2025 15:27
@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.09%. Comparing base (67d0e3c) to head (9c4934d).
⚠️ Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (67d0e3c) and HEAD (9c4934d). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (67d0e3c) HEAD (9c4934d)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8410       +/-   ##
===========================================
- Coverage   87.90%   67.09%   -20.82%     
===========================================
  Files        1951      816     -1135     
  Lines       76035    36404    -39631     
  Branches     1341      175     -1166     
===========================================
- Hits        66837    24424    -42413     
- Misses       8797    11923     +3126     
+ Partials      401       57      -344     
Flag Coverage Δ
integrationtests 63.95% <ø> (-0.02%) ⬇️
unittests 96.30% <100.00%> (+9.73%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 76.86% <ø> (-8.07%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.01% <ø> (-12.92%) ⬇️
dynamic_scheduler 96.30% <100.00%> (+0.02%) ⬆️
dynamic_sidecar 81.87% <ø> (-8.56%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 58.81% <ø> (-29.16%) ⬇️

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 67d0e3c...9c4934d. 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 23, 2025

🧪 CI Insights

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

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI unit-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

thx

@GitHK GitHK requested a review from pcrespov September 24, 2025 06:35
@GitHK GitHK enabled auto-merge (squash) September 24, 2025 07:08
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.

th

@sonarqubecloud
Copy link

@GitHK GitHK merged commit 129c290 into ITISFoundation:master Sep 24, 2025
92 of 95 checks passed
@GitHK GitHK deleted the pr-osparc-migrate-dy-scheduler-part3 branch September 24, 2025 08:37
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 24, 2025
65 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants