feat: add GET /run/list endpoint (#39)#279
feat: add GET /run/list endpoint (#39)#279saathviksheerla wants to merge 2 commits intoopenml:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #279 +/- ##
=======================================
Coverage ? 53.31%
=======================================
Files ? 35
Lines ? 1525
Branches ? 131
=======================================
Hits ? 813
Misses ? 710
Partials ? 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new OpenML runs router and registers it with the FastAPI app. Introduces a 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Using a request body (Body) for the GET /run/list parameters (including pagination) can be problematic for standard HTTP clients and tooling; consider using query parameters for the GET variant and keeping the body-based interface only for POST /run/list, while still sharing the same handler logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using a request body (Body) for the GET /run/list parameters (including pagination) can be problematic for standard HTTP clients and tooling; consider using query parameters for the GET variant and keeping the body-based interface only for POST /run/list, while still sharing the same handler logic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/routers/openml/runs.py (1)
107-124: Consider adding ORDER BY for deterministic pagination.The query lacks an ORDER BY clause, which means pagination results could be inconsistent across requests (same offset may return different rows). Adding
ORDER BY r.ridwould ensure stable, predictable pagination.♻️ Proposed fix
FROM run r JOIN algorithm_setup a ON r.setup = a.sid {where_clause} + ORDER BY r.rid LIMIT :limit OFFSET :offset🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routers/openml/runs.py` around lines 107 - 124, The SQL constructed in the variable query in runs.py uses LIMIT/OFFSET without an ORDER BY, causing non-deterministic pagination; update the text(...) SQL to include a deterministic ORDER BY clause (e.g., ORDER BY r.rid) placed before the LIMIT :limit OFFSET :offset so that queries built with where_clause and the query variable return stable, repeatable pages for functions that rely on run_id ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/routers/openml/runs.py`:
- Around line 107-124: The SQL constructed in the variable query in runs.py uses
LIMIT/OFFSET without an ORDER BY, causing non-deterministic pagination; update
the text(...) SQL to include a deterministic ORDER BY clause (e.g., ORDER BY
r.rid) placed before the LIMIT :limit OFFSET :offset so that queries built with
where_clause and the query variable return stable, repeatable pages for
functions that rely on run_id ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 215d38de-d419-460b-92fe-fa8de6ea3b0b
📒 Files selected for processing (3)
src/main.pysrc/routers/openml/runs.pytests/routers/openml/runs_list_test.py
1 participant
Description
Adds the
GET /run/listandPOST /run/listendpoints for listing runs with optional filters.Fixes: #39
Filters supported (all optional, combinable with AND logic):
run_id— list of run IDstask_id— list of task IDsflow_id— list of flow IDssetup_id— list of setup IDsuploader— list of user IDstag— single tag string (validated viaSystemString64)Pagination is supported via the nested
paginationbody parameter (consistent with other list endpoints).Returns 404 with code 372 when no runs match the filters.
Deviation from PHP: The PHP API requires at least one filter (error 510). This endpoint follows the pattern of other list endpoints in this API and allows no-filter requests, returning all runs paginated.
Checklist