Skip to content

🎨 introduce include_children query parameter for activity monitor / project activity listings (πŸ—ƒοΈ) #7718

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.

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

Merged
merged 13 commits into from
May 22, 2025

Conversation

matusdrobuliak66
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 commented May 21, 2025

What do these changes do?

  • 🎨 introduce include_children query parameter for activity monitor / project activity listings (πŸ—ƒοΈ)

Related issue/s

How to test

Dev-ops

@matusdrobuliak66 matusdrobuliak66 self-assigned this May 21, 2025
Copy link

codecov bot commented May 21, 2025

Codecov Report

Attention: Patch coverage is 67.44186% with 14 lines in your changes missing coverage. Please review.

Project coverage is 87.39%. Comparing base (9188df0) to head (5153f83).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7718      +/-   ##
==========================================
+ Coverage   87.19%   87.39%   +0.20%     
==========================================
  Files        1786     1753      -33     
  Lines       69761    68734    -1027     
  Branches     1203     1047     -156     
==========================================
- Hits        60827    60072     -755     
+ Misses       8606     8371     -235     
+ Partials      328      291      -37     
Flag Coverage Ξ”
integrationtests 64.51% <28.57%> (+0.11%) ⬆️
unittests 86.59% <67.44%> (+0.19%) ⬆️
Components Coverage Ξ”
api 76.84% <ΓΈ> (ΓΈ)
pkg_aws_library βˆ… <ΓΈ> (βˆ…)
pkg_dask_task_models_library βˆ… <ΓΈ> (βˆ…)
pkg_models_library 93.09% <100.00%> (+<0.01%) ⬆️
pkg_notifications_library 85.26% <ΓΈ> (ΓΈ)
pkg_postgres_database 88.61% <ΓΈ> (ΓΈ)
pkg_service_integration 69.92% <ΓΈ> (ΓΈ)
pkg_service_library 72.29% <0.00%> (-0.02%) ⬇️
pkg_settings_library βˆ… <ΓΈ> (βˆ…)
pkg_simcore_sdk 85.07% <ΓΈ> (ΓΈ)
agent 96.46% <ΓΈ> (ΓΈ)
api_server 91.59% <ΓΈ> (ΓΈ)
autoscaling 96.07% <ΓΈ> (ΓΈ)
catalog 92.70% <ΓΈ> (ΓΈ)
clusters_keeper 99.25% <ΓΈ> (ΓΈ)
dask_sidecar 91.67% <ΓΈ> (ΓΈ)
datcore_adapter 98.12% <ΓΈ> (ΓΈ)
director 76.78% <ΓΈ> (ΓΈ)
director_v2 91.04% <100.00%> (+<0.01%) ⬆️
dynamic_scheduler 96.76% <ΓΈ> (βˆ…)
dynamic_sidecar 90.18% <ΓΈ> (ΓΈ)
efs_guardian 89.79% <ΓΈ> (ΓΈ)
invitations 93.28% <ΓΈ> (ΓΈ)
payments 92.63% <ΓΈ> (ΓΈ)
resource_usage_tracker 89.02% <0.00%> (ΓΈ)
storage 87.63% <ΓΈ> (+0.14%) ⬆️
webclient βˆ… <ΓΈ> (βˆ…)
webserver 85.72% <68.75%> (+0.04%) ⬆️

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 9188df0...5153f83. 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.

@matusdrobuliak66 matusdrobuliak66 changed the title 🎨 WIP: MD 🎨 introduce include_children query parameter for activity monitor / project activity listings May 21, 2025
@matusdrobuliak66 matusdrobuliak66 changed the title 🎨 introduce include_children query parameter for activity monitor / project activity listings 🎨 introduce include_children query parameter for activity monitor / project activity listings May 21, 2025
@matusdrobuliak66 matusdrobuliak66 marked this pull request as ready for review May 21, 2025 13:55
@matusdrobuliak66 matusdrobuliak66 added this to the Bazinga! milestone May 21, 2025
@matusdrobuliak66 matusdrobuliak66 added a:webserver issue related to the webserver service a:director-v2 issue related with the director-v2 service labels May 21, 2025
Copy link
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.

πŸš€

@matusdrobuliak66 matusdrobuliak66 changed the title 🎨 introduce include_children query parameter for activity monitor / project activity listings 🎨 introduce include_children query parameter for activity monitor / project activity listings (πŸ—ƒοΈ) May 21, 2025
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.

tested πŸ‘Œ

@matusdrobuliak66 matusdrobuliak66 enabled auto-merge (squash) May 21, 2025 16:33
@matusdrobuliak66 matusdrobuliak66 added the πŸ€–-automerge marks PR as ready to be merged for Mergify label May 21, 2025
@matusdrobuliak66
Copy link
Collaborator Author

@mergify queue

Copy link
Contributor

mergify bot commented May 21, 2025

queue

🟠 Waiting for conditions to match

  • -closed [πŸ“Œ queue requirement]
  • -conflict [πŸ“Œ queue requirement]
  • -draft [πŸ“Œ queue requirement]
  • any of: [πŸ“Œ queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of: [πŸ”€ queue conditions]
    • all of: [πŸ“Œ queue conditions of queue default]
      • #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-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-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

@pcrespov pcrespov requested a review from Copilot May 21, 2025 19:18
Copy link
Contributor

@Copilot 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 introduces a new query parameter, include_children, to the activity monitor and project activity listings endpoints so that activities for child projects are optionally included. Key changes include:

  • Adding and exposing a new function to fetch project UUIDs by root parent project ID.
  • Updating computations endpoints and RPC calls to accept a list of project IDs instead of a single project ID.
  • Adjusting related tests, database queries, and error messages to support the new filtering functionality.

Reviewed Changes

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

Show a summary per file
File Description
services/web/server/src/simcore_service_webserver/projects/projects_metadata_service.py Updated imports and all to expose the new function.
services/web/server/src/simcore_service_webserver/projects/_metadata_service.py Added get_project_uuids_by_root_parent_project_id function.
services/web/server/src/simcore_service_webserver/projects/_metadata_repository.py Added database queries to support filtering by root parent project UUID.
services/web/server/src/simcore_service_webserver/director_v2/_controller/computations_rest.py Revised endpoint parameter types and query parsing to support include_children.
services/director-v2/_computations_service.py Added filtering logic for child projects and updated calls to RPC functions.
services/director-v2/api/rpc/_computations.py Changed API signatures from single project_id to list of project_ids and adjusted downstream calls.
Other files (tests, database migrations, and library updates) Adjusted tests and error messages, and added indexes to support efficient queries.
Files not reviewed (1)
  • services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml: Language not supported
Comments suppressed due to low confidence (2)

services/director-v2/api/rpc/_computations.py:157

  • There is a potential KeyError if a task’s project_uuid is not present in project_uuid_to_iteration; consider adding error handling or a default value.
project_uuid_to_iteration[task.project_uuid],

services/director-v2/_computations_service.py:141

  • [nitpick] Consider renaming 'child_projects_with_root' to 'project_ids_with_children' for improved clarity of the variable's purpose.
if include_children:

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.

There is I think a problem with your pydantic model.
Also I do not see any test that reflects these changes, is this coming afterwards?

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

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.

Somehow this was not submitted yesterday.

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 will not block you, but the only change in tests I see are these:
image

you still test the exact same thing. not multiple projects so I do not get where you test this change.

@matusdrobuliak66 matusdrobuliak66 merged commit 066b8a2 into ITISFoundation:master May 22, 2025
134 of 136 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:director-v2 issue related with the director-v2 service a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants