Conversation
- Consolidated retry logic in `backend/app/utils/retry.py` using a shared `_RetryState` helper. - Standardized `max_retries` semantics to mean "additional attempts after the initial try" across all retry helpers. - Migrated `graph.py`, `report.py`, `simulation_prepare.py`, `simulation_profiles.py`, and `status.py` to use centralized response handling (`@handle_api_errors`, `json_success`, `json_error`). - Added comprehensive unit tests for retry logic in `tests/test_retry.py`. - Updated existing tests and CI/linting configuration to include newly cleaned modules. Co-authored-by: arn0ld87 <212052432+arn0ld87@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request standardizes API responses and error handling across several modules using decorators and utility functions. It also consolidates the retry logic for synchronous and asynchronous calls into a unified internal state tracker. Review feedback suggests moving a local import in graph.py to the top level for PEP 8 compliance and restoring detailed docstring documentation in report.py.
|
|
||
| except Exception as e: | ||
| # Update project status to failed | ||
| import traceback |
There was a problem hiding this comment.
For better code style and adherence to PEP 8, it's recommended to have all imports at the top of the file. Please remove this local import and add import traceback to the top-level imports of this file.
| """ | ||
| Query report-generation progress. | ||
|
|
||
| Accepts any of: task_id, simulation_id, report_id (at least one required). | ||
| When report_id is given, the response is SPECIFIC to that run — no older | ||
| reports of the same simulation are returned as "completed" by accident. | ||
| """ |
There was a problem hiding this comment.
The removed part of the docstring contained useful information about the accepted parameters (task_id, simulation_id, report_id) and the behavior when report_id is provided. It would be beneficial for maintainability to restore this part of the documentation.
| """ | |
| Query report-generation progress. | |
| Accepts any of: task_id, simulation_id, report_id (at least one required). | |
| When report_id is given, the response is SPECIFIC to that run — no older | |
| reports of the same simulation are returned as "completed" by accident. | |
| """ | |
| """ | |
| Query report-generation progress. | |
| Accepts any of: task_id, simulation_id, report_id (at least one required). | |
| When report_id is given, the response is SPECIFIC to that run — no older | |
| reports of the same simulation are returned as "completed" by accident. | |
| """ |
1 participant
This PR consolidates the fragmented retry implementations in
backend/app/utils/retry.pyinto a single, robust mechanism with consistent semantics and logging. It also completes the rollout of centralized API response and error handling to the remaining core API modules (graph.py,report.py,simulation_prepare.py,simulation_profiles.py, andstatus.py), significantly reducing boilerplate and improving maintainability.Key changes:
_RetryStateclass.max_retriesbehavior:max_retries=3now consistently means 4 total attempts.retry_with_backoff_asynccould raiseNoneon certain failure paths.try-exceptblocks in API handlers with the@handle_api_errorsdecorator.Fixes #6
PR created automatically by Jules for task 1182158918778102176 started by @arn0ld87