Skip to content

🎨 Adds missing features to generic_scheduler #8490

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 9, 2025
Merged

🎨 Adds missing features to generic_scheduler #8490

merged 9 commits into from
Oct 9, 2025

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Oct 9, 2025 •
edited
Loading

What do these changes do?

In order to write the scheduler there are some required missing features that have been added:

  • schedule_id is now part of the OperationContext and can be required both during execute and revert.
  • an event can now be set to None, basically unregistering it
  • OperationName can now be recovered form a ScheduleId

Related issue/s

How to test

Dev-ops

@GitHK GitHK added this to the Cheops milestone Oct 9, 2025
@GitHK GitHK self-assigned this Oct 9, 2025
@GitHK GitHK changed the title Adds missing features 🎨 Adds missing features to generic_scheduler Oct 9, 2025
@GitHK GitHK requested a review from Copilot October 9, 2025 08:46
Copilot AI reviewed Oct 9, 2025
View reviewed changes
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 enhances the generic scheduler by adding missing features required for scheduler functionality. The changes focus on integrating schedule IDs into operation context, enabling event unregistration, and providing operation name lookup capabilities.

  • Added schedule_id as a reserved context key automatically provided during operation execution
  • Enhanced event management to support unregistering events by setting them to None
  • Added functionality to retrieve operation names from schedule IDs

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
_models.py Defines reserved context keys enum and validation set
_operation.py Adds validation to prevent steps from providing reserved context keys
_core.py Implements schedule ID injection and operation name lookup functionality
_event_after.py Enables event unregistration when to_start is None
_event_after_registration.py Updates function signatures to accept None for operation unregistration
__init__.py Exports new get_operation_name function
Test files Comprehensive test coverage for all new features

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

@GitHK GitHK requested a review from Copilot October 9, 2025 08:51
Copilot AI reviewed Oct 9, 2025
View reviewed changes
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

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

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

@codecov
Copy link

codecov bot commented Oct 9, 2025 •
edited
Loading

Codecov Report

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

❗ There is a different number of reports uploaded between BASE (3319147) and HEAD (df32cb2). Click for more details.

HEAD has 30 uploads less than BASE
Flag BASE (3319147) HEAD (df32cb2)
unittests 31 1
Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #8490       +/-   ##
===========================================
- Coverage   87.28%   68.14%   -19.14%     
===========================================
  Files        1999      854     -1145     
  Lines       77654    37859    -39795     
  Branches     1333      175     -1158     
===========================================
- Hits        67778    25800    -41978     
- Misses       9477    12002     +2525     
+ Partials      399       57      -342
Flag Coverage Δ
integrationtests 64.12% <ø> (-0.05%) ⬇️
unittests 96.91% <100.00%> (+10.70%) ⬆️
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.93% <ø> (-8.03%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.14% <ø> (-12.83%) ⬇️
dynamic_scheduler 96.91% <100.00%> (+0.06%) ⬆️
dynamic_sidecar 81.87% <ø> (ø)
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.11% <ø> (-28.24%) ⬇️

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 3319147...df32cb2. 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.

@GitHK GitHK marked this pull request as ready for review October 9, 2025 09:12
@mergify
Copy link
Contributor

mergify bot commented Oct 9, 2025 •
edited
Loading

🧪 CI Insights

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

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI system-tests Base branch is broken, but retries were needed. Could be early signs of flakiness 👀 Broken 1 View View

pcrespov
pcrespov approved these changes Oct 9, 2025
View reviewed changes
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.

thx

giancarloromeo
giancarloromeo approved these changes Oct 9, 2025
View reviewed changes
Copy link
Contributor

@giancarloromeo giancarloromeo left a comment

Choose a reason for hiding this comment

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

👌

@GitHK GitHK enabled auto-merge (squash) October 9, 2025 13:23
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 9, 2025

Quality Gate Passed 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

@GitHK GitHK merged commit 98bd68a into ITISFoundation:master Oct 9, 2025
92 of 95 checks passed
@GitHK GitHK deleted the pr-osparc-migrate-dy-scheduler-part8 branch October 9, 2025 13:50
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Oct 28, 2025
Open
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@giancarloromeo giancarloromeo giancarloromeo approved these changes

@pcrespov pcrespov pcrespov approved these changes

@matusdrobuliak66 matusdrobuliak66 Awaiting requested review from matusdrobuliak66

@bisgaard-itis bisgaard-itis Awaiting requested review from bisgaard-itis

Assignees

@GitHK GitHK

Labels

a:dynamic-scheduler

Projects

None yet

Milestone

Cheops

Development

Successfully merging this pull request may close these issues.

3 participants

@GitHK @giancarloromeo @pcrespov