🐛♻️ Fixes public-api pagination issues and overall normalization & documentation #7747
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pcrespov
requested review from
bisgaard-itis,
Copilot,
matusdrobuliak66,
mguidon and
sanderegg
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the pagination logic in the GET /solvers/pages endpoint and updates the SERVICES.md documentation with auto‐generated service badge information.
- Fixes wrong pagination by enforcing that page parameters match the expected metadata and using the correct total count.
- Updates SERVICES.md to include auto-generated badges and links for the various services.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| services/api-server/src/simcore_service_api_server/api/routes/solvers.py | Adjusts pagination logic by replacing assignments with assertions and using page_meta.total for accurate counting. |
| SERVICES.md | Introduces an auto-generated file with updated service badges and links for documentation. |
services/api-server/src/simcore_service_api_server/api/routes/solvers.py
Show resolved
Hide resolved
services/api-server/src/simcore_service_api_server/api/routes/solvers.py
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7747 +/- ##
===========================================
+ Coverage 76.66% 87.28% +10.61%
===========================================
Files 1738 1814 +76
Lines 66459 70740 +4281
Branches 1214 1214
===========================================
+ Hits 50953 61745 +10792
+ Misses 15178 8652 -6526
- Partials 328 343 +15
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
sanderegg
approved these changes
May 23, 2025
pcrespov
force-pushed
the
fix/public-api-solvers-pagination
branch
from
9726279 to
35daa2b
Compare
pcrespov
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes pagination issues in the public API by aligning pagination limits between endpoints and refactoring how pagination parameters are handled.
- Fixes wrong pagination parameters in GET /solvers/page endpoints.
- Refactors pagination arguments to accept None defaults and use unified type annotations.
- Updates tests and documentation to reflect these changes.
Reviewed Changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| services/api-server/tests/unit/api_solvers/test_api_routers_solvers.py | Adds new tests for pagination links and last page behavior |
| services/api-server/src/simcore_service_api_server/services_rpc/catalog.py | Refactors pagination metadata extraction via a helper function and adjusts parameter types |
| services/api-server/src/simcore_service_api_server/services_http/webserver.py | Updates type annotations for pagination in HTTP endpoints |
| services/api-server/src/simcore_service_api_server/models/pagination.py | Updates REST pagination type definitions and limits |
| services/api-server/src/simcore_service_api_server/api/routes/solvers.py | Changes default pagination limits and adjusts page totals in responses |
| Other files | Similar refactoring of pagination argument names and types, with updates in tests and documentation |
Files not reviewed (3)
- services/web/server/VERSION: Language not supported
- services/web/server/setup.cfg: Language not supported
- services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml: Language not supported
Comments suppressed due to low confidence (2)
services/api-server/tests/unit/api_solvers/test_api_routers_solvers.py:215
- The error message duplicates 'prev' in its description. Consider updating it to uniquely list all required pagination links (e.g. 'prev', 'self', and 'first' links) to avoid confusion.
assert (total_items > 1), "Total items in MOCK examples should be greater than 1 for pagination test since we need 'prev', 'self' and 'prev' links"
services/api-server/src/simcore_service_api_server/api/routes/solvers.py:139
- Changing the pagination limit from DEFAULT_PAGINATION_LIMIT to MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE in the iteration may affect how many items are processed per page. Please confirm that this change is intentional and that downstream consumers are compatible with this higher limit.
for page_params in iter_pagination_params(limit=MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE):
matusdrobuliak66
approved these changes
May 24, 2025
sanderegg
approved these changes
May 25, 2025
Member
sanderegg
left a comment
sanderegg
left a comment
There was a problem hiding this comment.
Looking good, but please double check the paths if they really match.
Also why do the pagination offsets limits have to be aligned?
pcrespov
force-pushed
the
fix/public-api-solvers-pagination
branch
from
24838cd to
165b25c
Compare
GitHK
approved these changes
May 26, 2025
… and update tests for last page pagination
…e services and routes
…_PAGE directly in pagination logic
pcrespov
force-pushed
the
fix/public-api-solvers-pagination
branch
from
4facc7a to
9609abc
Compare
Member
Author
|
@mergify queue |
Contributor
🟠 Waiting for conditions to match
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
GET /solvers/pages(and other endpoints)PageLimitIntupper limit was set strictlyNonepagination arguments to delegate the size of the page (i.e.limit) to the default downstreamSERVICES.mdshows info on openapi for each service using swagger/redoc badgesRelated issue/s
How to test
Dev-ops