Feat: Add Report Repository for Async Operations #66
Conversation
WalkthroughMigrates DB access from synchronous SQLAlchemy to async APIs (async engine, async sessionmaker, AsyncSession, async get_db) and adds an asynchronous ReportRepository with methods to create reports, update status, store partial results, save final reports, and fetch report state. Config adds async-capable Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant API as API / Service
participant Repo as ReportRepository
participant Sess as AsyncSessionLocal
participant DB as Database
Note over API,Repo: Async request flow
API->>Repo: await create_report_entry(report_id)
Repo->>Sess: async with AsyncSessionLocal() as session
Sess->>DB: INSERT Report
Sess->>DB: INSERT ReportState (PENDING)
DB-->>Sess: inserted rows
Sess->>Repo: commit & return Report
API->>Repo: await update_report_status(report_id, status)
Repo->>Sess: async with AsyncSessionLocal() as session
Sess->>DB: UPDATE ReportState ... RETURNING *
DB-->>Sess: updated row
Sess->>Repo: commit & return ReportState
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
backend/app/db/repositories/report_repository.py (2)
28-33: Add rollback on error for transaction safety.This method lacks error handling with the same issue as
update_report_status. Session cleanup is needed if the commit fails.Apply this diff:
async def store_partial_report_results(self, report_id: str, partial_data: Dict[str, Any]) -> ReportState | None: - stmt = update(ReportState).where(ReportState.report_id == report_id).values(partial_agent_output=partial_data).returning(ReportState) - result = await self.session.execute(stmt) - updated_report_state = result.scalar_one_or_none() - await self.session.commit() - return updated_report_state + try: + stmt = update(ReportState).where(ReportState.report_id == report_id).values(partial_agent_output=partial_data).returning(ReportState) + result = await self.session.execute(stmt) + updated_report_state = result.scalar_one_or_none() + await self.session.commit() + return updated_report_state + except Exception: + await self.session.rollback() + raise
35-40: Add rollback on error for transaction safety.This method also lacks error handling with the same concerns as the other update methods.
Apply this diff:
async def save_final_report(self, report_id: str, data: Dict[str, Any]) -> ReportState | None: - stmt = update(ReportState).where(ReportState.report_id == report_id).values(final_report_json=data, status=ReportStatusEnum.COMPLETED).returning(ReportState) - result = await self.session.execute(stmt) - updated_report_state = result.scalar_one_or_none() - await self.session.commit() - return updated_report_state + try: + stmt = update(ReportState).where(ReportState.report_id == report_id).values(final_report_json=data, status=ReportStatusEnum.COMPLETED).returning(ReportState) + result = await self.session.execute(stmt) + updated_report_state = result.scalar_one_or_none() + await self.session.commit() + return updated_report_state + except Exception: + await self.session.rollback() + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/db/database.py(1 hunks)backend/app/db/repositories/report_repository.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/db/repositories/report_repository.py (2)
backend/app/db/models/report.py (1)
Report(4-6)backend/app/db/models/report_state.py (2)
ReportState(17-26)ReportStatusEnum(8-12)
🪛 Ruff (0.14.5)
backend/app/db/database.py
24-24: Redefinition of unused Base from line 3
(F811)
24-24: Undefined name declarative_base
(F821)
🔇 Additional comments (3)
backend/app/db/database.py (1)
26-28: LGTM!The async context manager pattern is correctly implemented for yielding database sessions.
backend/app/db/repositories/report_repository.py (2)
7-9: LGTM!The repository initialization correctly stores the AsyncSession for use in async database operations.
42-45: LGTM!The read-only query is correctly implemented using async select with proper result handling.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
backend/app/core/__pycache__/config.cpython-313.pycis excluded by!**/*.pycbackend/app/db/__pycache__/connection.cpython-313.pycis excluded by!**/*.pycbackend/app/db/tests/__pycache__/test_connection.cpython-313-pytest-8.2.0.pycis excluded by!**/*.pycbackend/logs/app.logis excluded by!**/*.log
📒 Files selected for processing (3)
backend/app/core/config.py(2 hunks)backend/app/db/database.py(1 hunks)backend/app/db/repositories/report_repository.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/db/repositories/report_repository.py (2)
backend/app/db/models/report.py (1)
Report(4-6)backend/app/db/models/report_state.py (2)
ReportState(17-26)ReportStatusEnum(8-12)
🪛 Ruff (0.14.5)
backend/app/db/repositories/report_repository.py
20-20: Consider moving this statement to an else block
(TRY300)
41-41: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (6)
backend/app/core/config.py (2)
9-9: Async driver correctly configured.The DATABASE_URL now uses
sqlite+aiosqlite://which is the correct async driver for SQLite when usingcreate_async_engine(). This resolves the previous concern about async compatibility.
26-38: LGTM!The new database configuration fields provide flexible support for PostgreSQL connections and test database setup. The optional nature of these fields allows the application to fall back to the DATABASE_URL when not specified.
backend/app/db/database.py (1)
1-28: LGTM! Async migration correctly implemented.The migration from synchronous to asynchronous SQLAlchemy is well-executed:
- Correct async imports and engine creation
- Properly configured
AsyncSessionLocalwithexpire_on_commit=Falseto prevent detached instance errors after commitget_db()correctly uses async context manager pattern- All previous concerns from past reviews have been addressed
backend/app/db/repositories/report_repository.py (3)
12-33: LGTM! Robust error handling implemented.The method now includes comprehensive error handling with rollback on failure and graceful handling of duplicate keys by fetching the existing report. The try/except structure properly addresses the concerns raised in previous reviews.
Note: The static analysis hint (TRY300) about moving line 20 to an else block is a minor style preference and can be safely ignored.
35-44: LGTM! Error handling correctly added.The method now includes proper exception handling with rollback, addressing the concern from previous reviews. The transaction safety is ensured.
60-63: LGTM!This read-only method doesn't require the same level of error handling as the write operations since it performs no state mutations or commits. Allowing exceptions to propagate naturally is acceptable for query operations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/app/db/repositories/report_repository.py (1)
68-71: Consider error handling for consistency.While this read-only method doesn't require rollback, adding a try/except block would provide consistency with other repository methods and allow for more graceful error handling (e.g., logging, custom exceptions).
Optional enhancement:
async def get_report_state(self, report_id: str) -> ReportState | None: - stmt = select(ReportState).where(ReportState.report_id == report_id) - result = await self.session.execute(stmt) - return result.scalar_one_or_none() + try: + stmt = select(ReportState).where(ReportState.report_id == report_id) + result = await self.session.execute(stmt) + return result.scalar_one_or_none() + except Exception: + # Could add logging or custom exception handling here + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/app/core/__pycache__/config.cpython-313.pycis excluded by!**/*.pycbackend/logs/app.logis excluded by!**/*.log
📒 Files selected for processing (1)
backend/app/db/repositories/report_repository.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/db/repositories/report_repository.py (2)
backend/app/db/models/report.py (1)
Report(4-6)backend/app/db/models/report_state.py (2)
ReportState(17-26)ReportStatusEnum(8-12)
🪛 Ruff (0.14.5)
backend/app/db/repositories/report_repository.py
20-20: Consider moving this statement to an else block
(TRY300)
41-41: Consider moving this statement to an else block
(TRY300)
52-52: Consider moving this statement to an else block
(TRY300)
63-63: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (5)
backend/app/db/repositories/report_repository.py (5)
1-10: LGTM! Clean repository setup.The imports and class initialization follow best practices for async SQLAlchemy repositories with proper dependency injection.
12-33: Excellent error handling with idempotency.The IntegrityError handling properly implements idempotency by fetching and returning an existing report when a duplicate is detected. The rollback logic on all error paths ensures session consistency, addressing the previous review concern.
35-44: Transaction safety properly implemented.The try/except block with rollback ensures the session remains consistent if the commit fails, addressing the previous review feedback.
46-55: Consistent error handling applied.The method now matches the transaction safety pattern of other methods with proper rollback on failure, resolving the previous concern.
57-66: Transaction safety and atomic update correctly implemented.The error handling is consistent with other repository methods, and atomically updating both the final report data and status ensures data integrity.
|
the async methods in |
Overview: This PR introduces a new
report_repository.pyto centralize and manage asynchronous database operations related to reports.Changes
app/db/repositories/report_repository.pyto encapsulate report data access logic.save_final_report(report_id, data)for intuitive interaction.Summary by CodeRabbit
Infrastructure Updates
New Features
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.