Skip to content

Conversation

@wvangeit
Copy link
Contributor

@wvangeit wvangeit commented Aug 4, 2025

What do these changes do?

This adds some filters to the function jobs listings, and add pagination to the endpoint that lists the jobs of the job collection.
Originally my idea was to have a response_type at the latter endpoint that would allow for list/page, but didn't work out because of the pagination library we use. So another /page endpoint was added instead.

This PR also starts a service layer in the API server for functions.

Related issue/s

How to test

Run api server and web server tests.
Call function job listings with filters.

Dev-ops

No changes

@wvangeit wvangeit added this to the Voyager milestone Aug 4, 2025
@wvangeit wvangeit requested a review from bisgaard-itis August 4, 2025 14:52
@wvangeit wvangeit self-assigned this Aug 4, 2025
@wvangeit wvangeit requested a review from pcrespov as a code owner August 4, 2025 14:52
@wvangeit wvangeit added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Aug 4, 2025
@wvangeit wvangeit added the a:apiserver api-server service label Aug 4, 2025
@codecov
Copy link

codecov bot commented Aug 4, 2025

Codecov Report

❌ Patch coverage is 98.86364% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.97%. Comparing base (19e3d7a) to head (18de9fa).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8187      +/-   ##
==========================================
- Coverage   88.04%   87.97%   -0.08%     
==========================================
  Files        1907     1486     -421     
  Lines       73285    60930   -12355     
  Branches     1301      651     -650     
==========================================
- Hits        64526    53601   -10925     
+ Misses       8371     7099    -1272     
+ Partials      388      230     -158     
Flag Coverage Δ
integrationtests 64.35% <0.00%> (-0.05%) ⬇️
unittests 86.33% <98.86%> (-0.35%) ⬇️
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 71.69% <ø> (ø)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.05% <ø> (ø)
agent 93.81% <ø> (ø)
api_server 93.20% <98.79%> (+0.11%) ⬆️
autoscaling 95.89% <ø> (ø)
catalog 92.34% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 91.81% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 76.14% <ø> (-0.10%) ⬇️
director_v2 90.92% <ø> (-0.07%) ⬇️
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.12% <ø> (ø)
efs_guardian 89.60% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.60% <ø> (ø)
resource_usage_tracker 92.50% <ø> (+0.31%) ⬆️
storage 86.75% <ø> (+0.16%) ⬆️
webclient ∅ <ø> (∅)
webserver 88.18% <100.00%> (+<0.01%) ⬆️

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 19e3d7a...18de9fa. 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.

@wvangeit wvangeit added the 🤖-automerge marks PR as ready to be merged for Mergify label Aug 5, 2025
@wvangeit
Copy link
Contributor Author

wvangeit commented Aug 5, 2025

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Aug 5, 2025

queue

🛑 Configuration not compatible with a branch protection setting

The branch protection setting Require branches to be up to date before merging is not compatible with max_parallel_checks>1, queue_conditions != merge_conditions and must be unset.

@wvangeit wvangeit requested a review from Copilot August 5, 2025 13:15

This comment was marked as outdated.

@wvangeit wvangeit requested a review from Copilot August 5, 2025 13:19

This comment was marked as outdated.

@wvangeit wvangeit requested a review from Copilot August 5, 2025 13:22
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 filtering capabilities to function job listings and introduces pagination for job collection endpoints. It establishes a service layer pattern in the API server for functions to better encapsulate business logic.

  • Adds new filters for function jobs (by function ID, job IDs, and collection ID)
  • Introduces separate /page and /list endpoints for function job collections to support both paginated and non-paginated responses
  • Creates service layer classes (FunctionService and FunctionJobService) to abstract RPC client interactions

Reviewed Changes

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

Show a summary per file
File Description
_functions_service.py Adds new filter parameters to function job listing service method
_functions_repository.py Implements database filtering logic with proper SQL condition building
_functions_rpc.py Passes through new filter parameters to service layer
function_jobs_routes.py Updates endpoint to use new service layer and filter dependencies
function_job_collections_routes.py Adds separate /page and /list endpoints for different response formats
functions_filters.py Defines new filter schema for function job queries
_service_functions.py New service class for function-related operations
_service_function_jobs.py New service class for function job operations
Test files Updates tests to use new service patterns and verify filter functionality

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. left some suggestions.

It is important the @bisgaard-itis learns about this change. thx

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.

Thanks, just a minor thing

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.

Thanks a lot. A few questions/suggestions from my side.

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.

Thanks! added a few comments

@sonarqubecloud
Copy link

@mergify
Copy link
Contributor

mergify bot commented Aug 13, 2025

🧪 CI Insights

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

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on base branch Retries 🔍 CI Insights 📄 Logs
CI [sys] e2e (3.11, 14, ubuntu-24.04) Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
[unit] clusters-keeper (3.11, ubuntu-24.04) Base branch is healthy, but retries were needed. Could be early signs of flakiness 👀 Healthy 1 View View
system-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
unit-tests Base branch is healthy, but retries were needed. Could be early signs of flakiness 👀 Healthy 1 View View

@wvangeit
Copy link
Contributor Author

@mergify refresh

@mergify
Copy link
Contributor

mergify bot commented Aug 13, 2025

refresh

✅ Pull request refreshed

@wvangeit
Copy link
Contributor Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Aug 13, 2025

queue

🛑 Configuration not compatible with a branch protection setting

The branch protection setting Require branches to be up to date before merging is not compatible with max_parallel_checks>1, queue_conditions != merge_conditions and must be unset.

@wvangeit wvangeit merged commit 8548b0e into ITISFoundation:master Aug 13, 2025
145 of 148 checks passed
@wvangeit wvangeit deleted the add_function_job_filters branch August 13, 2025 08:25
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 2, 2025
61 tasks
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:apiserver api-server service a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants