Skip to content

Conversation

@mrnicegyu11
Copy link
Member

What do these changes do?

I found this by chance while checking some things for the trace-propagation in dv2 (this PR is unrelated to tracing). There is the param skip_observation_recreation which is boolean, and has impact on the behavior of dv2 dy service scheduling

Before, the param would be implicitly False by default, except for 1 case where it is overwritten to True (here https://github.com/mrnicegyu11/osparc-simcore/blob/ae88c2af1a5b4cb52f383ce6117718a1e1cfd522/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_observer.py#L60 ).
Now, for the few occurances of it, the param is always passed explicitly. This makes the behaviour more understandable when reading code.

A very minor change that I suggest, without impact on the behavior :--)

Related issue/s

How to test

Dev-ops

@mrnicegyu11 mrnicegyu11 added this to the Delirium Noel milestone Dec 2, 2025
@mrnicegyu11 mrnicegyu11 self-assigned this Dec 2, 2025
@mrnicegyu11 mrnicegyu11 added t:maintenance Some planned maintenance work a:director-v2 issue related with the director-v2 service labels Dec 2, 2025
@mrnicegyu11 mrnicegyu11 marked this pull request as ready for review December 2, 2025 15:12
@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.46%. Comparing base (38f7907) to head (ef2ee70).

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

HEAD has 31 uploads less than BASE
Flag BASE (38f7907) HEAD (ef2ee70)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8660       +/-   ##
===========================================
- Coverage   87.55%   68.46%   -19.09%     
===========================================
  Files        2019      782     -1237     
  Lines       79254    35552    -43702     
  Branches     1386      154     -1232     
===========================================
- Hits        69393    24342    -45051     
- Misses       9465    11161     +1696     
+ Partials      396       49      -347     
Flag Coverage Δ
integrationtests 63.89% <100.00%> (-0.07%) ⬇️
unittests 84.75% <100.00%> (-1.63%) ⬇️
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 75.55% <ø> (-8.96%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 91.23% <100.00%> (-0.03%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 82.00% <ø> (-8.83%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 58.84% <ø> (-27.85%) ⬇️

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 38f7907...ef2ee70. 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 Dec 2, 2025

🧪 CI Insights

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

🟢 All jobs passed!

But CI Insights is watching 👀

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.

I hate defaults

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

Please make sure to also check that all tests are greeen, then I will have a look at it again to see if other things break

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:director-v2 issue related with the director-v2 service t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants