Sort jobs by creation in API #5545
Conversation
📝 WalkthroughWalkthroughThe changes enhance job management and sorting within the supervisor's job system. A Changes
Sequence DiagramsequenceDiagram
participant Job as SupervisorJob
participant JobManager as SystemJobs
participant API as APIJobs
Job->>Job: Create with timestamp
Job->>JobManager: Add job to job list
JobManager->>JobManager: Sort jobs by creation time
JobManager->>API: Provide sorted job list
API->>API: Process jobs in chronological order
API-->>Client: Return jobs (newest first)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (3)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/api/test_jobs.py (1)
239-319: Strengthen the job sorting test.While the test verifies the structure of sorted jobs, it could be more robust by:
- Adding explicit delays between job creation to ensure distinct timestamps
- Verifying that the created timestamps are actually in descending order
async def test_jobs_sorted(api_client: TestClient, coresys: CoreSys): """Test jobs are sorted by datetime in results.""" test = TestClass(coresys) await test.test_jobs_sorted_1() + await asyncio.sleep(0.1) # Ensure distinct timestamps await test.test_jobs_sorted_2() resp = await api_client.get("/jobs/info") result = await resp.json() + + # Verify descending order of creation timestamps + jobs = result["data"]["jobs"] + timestamps = [job["created"] for job in jobs] + assert timestamps == sorted(timestamps, reverse=True), "Jobs not in descending order" + assert result["data"]["jobs"] == [
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
supervisor/api/jobs.py(2 hunks)supervisor/jobs/__init__.py(3 hunks)tests/api/test_jobs.py(6 hunks)
🔇 Additional comments (5)
supervisor/api/jobs.py (2)
34-40: LGTM! Clear documentation of job ordering logic.The docstring clearly explains the rationale behind job ordering, both for child jobs (chronological) and the main list (reverse chronological).
65-65: LGTM! Correct implementation of reverse chronological ordering.Using
insert(0, job_dict)effectively places newer jobs at the start of the list, which aligns with the goal of prioritizing recent jobs in the API response.supervisor/jobs/__init__.py (2)
87-88: LGTM! Well-implemented creation timestamp field.The
createdfield is correctly implemented with:
- Immutable value (frozen)
- Automatic initialization (factory=utcnow)
- Proper placement before UUID field
124-124: LGTM! Proper ISO format for timestamp serialization.The created timestamp is correctly serialized using ISO 8601 format, which is the standard for date/time representation in APIs.
tests/api/test_jobs.py (1)
Line range hint
105-189: LGTM! Test assertions properly updated for new field.Existing test assertions correctly updated to expect the created field, using ANY matcher appropriately for timestamp values.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
supervisor/api/jobs.py(2 hunks)supervisor/jobs/__init__.py(3 hunks)tests/api/test_jobs.py(6 hunks)tests/jobs/test_job_decorator.py(1 hunks)tests/jobs/test_job_manager.py(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- supervisor/api/jobs.py
- supervisor/jobs/init.py
🧰 Additional context used
🪛 GitHub Check: Run tests Python 3.12.8
tests/api/test_jobs.py
[failure] 273-273:
test_jobs_sorted
AssertionError: assert [{'child_jobs...rs': [], ...}] == [{'child_jobs...rs': [], ...}]
At index 1 diff: {'name': 'test_jobs_sorted_1', 'reference': None, 'uuid': '570cfe0680bb444ab1247884f4d6f6c8', 'progress': 0, 'stage': None, 'done': True, 'errors': [], 'created': '2025-01-14T22:04:50.304738+00:00', 'child_jobs': [{'name': 'test_jobs_sorted_inner_2', 'reference': None, 'uuid': '5b05fde09cba4b73abcdf69e32107cd8', 'progress': 0, 'stage': None, 'done': True, 'errors': [], 'created': '2025-01-14T22:04:50.305044+00:00', 'child_jobs': []}, {'name': 'test_jobs_sorted_inner_1', 'reference': None, 'uuid': 'b0e946d7a2474f75b1a5181a90fd4add', 'progress': 0, 'stage': None, 'done': True, 'errors': [], 'created': '2025-01-14T22:04:50.304903+00:00', 'child_jobs': []}]} != {'created': , 'name': 'test_jobs_sorted_1', 'reference': None, 'uuid': , 'progress': 0, 'stage': None, 'done': True, 'errors': [], 'child_jobs': [{'created': , 'name': 'test_jobs_sorted_inner_1', 'reference': None, 'uuid': , 'progress': 0, 'stage': None, 'done': True, 'errors': [], 'child_jobs': []}, {'created': , 'name': 'test_jobs_sorted_inner_2', 'reference': None, 'uuid': , 'progress': 0, 'stage': None, 'done': True, 'errors': [], 'child_jobs': []}]}
Full diff:
[
{
'child_jobs': [],
-
'created': <ANY>,
-
'created': '2025-01-14T22:04:50.305231+00:00', 'done': True, 'errors': [], 'name': 'test_jobs_sorted_2', 'progress': 0, 'reference': None, 'stage': None,
-
'uuid': <ANY>,
-
'uuid': '0c7ea0d934d443609fa2e447df8ec7ac', }, { 'child_jobs': [ { 'child_jobs': [],
-
'created': <ANY>,
-
'created': '2025-01-14T22:04:50.305044+00:00', -
'done': True, -
'errors': [], -
'name': 'test_jobs_sorted_inner_2', -
'progress': 0, -
'reference': None, -
'stage': None, -
'uuid': '5b05fde09cba4b73abcdf69e32107cd8', -
}, -
{ -
'child_jobs': [], -
'created': '2025-01-14T22:04:50.304903+00:00', 'done': True, 'errors': [], 'name': 'test_jobs_sorted_inner_1', 'progress': 0, 'reference': None, 'stage': None, -
'uuid': 'b0e946d7a2474f75b1a5181a90fd4add',
-
'uuid': <ANY>, -
}, -
{ -
'child_jobs': [], -
'created': <ANY>, -
'done': True, -
'errors': [], -
'name': 'test_jobs_sorted_inner_2', -
'progress': 0, -
'reference': None, -
'stage': None, -
'uuid': <ANY>, }, ], -
'created': <ANY>,
-
'created': '2025-01-14T22:04:50.304738+00:00', 'done': True, 'errors': [], 'name': 'test_jobs_sorted_1', 'progress': 0, 'reference': None, 'stage': None,
-
'uuid': <ANY>,
-
]
'uuid': '570cfe0680bb444ab1247884f4d6f6c8', },
🔇 Additional comments (3)
tests/jobs/test_job_manager.py (1)
107-107: LGTM! Test assertions updated for new job field.The test assertions have been consistently updated to include the new
createdfield in job data structures.Also applies to: 129-129, 151-151, 173-173, 200-200, 226-226
tests/api/test_jobs.py (1)
239-241: LGTM! New test added for job sorting functionality.The test verifies that jobs are sorted by creation datetime in the API response, which aligns with the PR objective.
tests/jobs/test_job_decorator.py (1)
1000-1000: LGTM! Test assertion updated for new job field.The test assertion has been updated to include the new
createdfield in job data structure.
Proposed change
Add create date time field to jobs in API and sort them based on that. Probably not needed anymore for cloud backup but seemed to make the job list API more useful so thought I'd add it anyway.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests
These changes improve the job management system by providing more precise and time-aware job tracking and presentation.