Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Jul 29, 2025

What do these changes do?

Notify project_id instead of node_id when the status of a node changes. Allows for multiple users to view the node's status change.

Related issue/s

How to test

  • run locally, services open as usual

Dev-ops

@GitHK GitHK requested a review from Copilot July 29, 2025 12:56
@GitHK GitHK self-assigned this Jul 29, 2025
@GitHK GitHK added this to the Engage milestone Jul 29, 2025

This comment was marked as outdated.

@GitHK GitHK requested a review from Copilot July 29, 2025 12:59

This comment was marked as outdated.

@GitHK GitHK requested a review from Copilot July 29, 2025 13:01

This comment was marked as outdated.

@GitHK GitHK requested a review from Copilot July 29, 2025 13:03
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 changes the notification mechanism for dynamic service status updates from notifying individual users to notifying all users within a project. This enables multiple users in the same project to receive real-time status updates when a node's status changes.

  • Changed notification target from user_id to project_id in the status monitoring system
  • Updated the service tracker API to retrieve project IDs instead of user IDs
  • Modified the notifier to emit status changes to project-based socket rooms

Reviewed Changes

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

File Description
_deferred_get_status.py Updated status notification logic to use project_id instead of user_id
_api.py Renamed function from get_user_id_for_service to get_project_id_for_service and updated return type
__init__.py Updated module exports to reflect the API function name change
_notifier.py Changed notification parameters and socket room targeting from user-based to project-based

@codecov
Copy link

codecov bot commented Jul 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.79%. Comparing base (68679ab) to head (584764e).
⚠️ Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (68679ab) and HEAD (584764e). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (68679ab) HEAD (584764e)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8175       +/-   ##
===========================================
- Coverage   88.04%   67.79%   -20.26%     
===========================================
  Files        1900      787     -1113     
  Lines       73074    35092    -37982     
  Branches     1280      176     -1104     
===========================================
- Hits        64336    23789    -40547     
- Misses       8358    11245     +2887     
+ Partials      380       58      -322     
Flag Coverage Δ
integrationtests 64.11% <ø> (+0.02%) ⬆️
unittests 96.27% <100.00%> (+9.59%) ⬆️
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 77.28% <ø> (-7.83%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 77.11% <ø> (-13.89%) ⬇️
dynamic_scheduler 96.27% <100.00%> (ø)
dynamic_sidecar 87.31% <ø> (-2.77%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.08% <ø> (-28.97%) ⬇️

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 68679ab...584764e. 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 July 29, 2025 13:28
@GitHK GitHK added the 🤖-automerge marks PR as ready to be merged for Mergify label Jul 29, 2025
@GitHK
Copy link
Contributor Author

GitHK commented Jul 29, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 29, 2025

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 = deploy to dockerhub
        • check-skipped = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-neutral = system-tests
        • check-skipped = system-tests
        • check-success = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-neutral = integration-tests
        • check-skipped = integration-tests
        • check-success = integration-tests

@mergify
Copy link
Contributor

mergify bot commented Jul 29, 2025

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.

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

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

project_in instead of user_id, thanks!

@sonarqubecloud
Copy link

@odeimaiz odeimaiz merged commit 5f20287 into ITISFoundation:master Jul 30, 2025
93 of 96 checks passed
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:dynamic-scheduler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants