Draft: Refactor(sms_service): migrate to httpx
Merge Request
Overview
Refactors the SMS service to use asynchronous HTTP requests using httpx instead of the synchronous requests library. This improves performance by preventing blocking calls in the FastAPI event loop.
What does this MR do and why?
The existing SMS service used the synchronous requests library, which blocks the FastAPI event loop during external API calls. This can degrade performance under concurrent load.
This MR replaces requests with httpx.AsyncClient and updates the service to be asynchronous. The change ensures non-blocking behavior while keeping the overall architecture and functionality unchanged.
No business logic or API contract has been modified. The flow remains: Generate OTP → Store in DB → Send SMS
Changes Made
- Modified app/services/sms_service.py
- Replaced requests with httpx
- Converted send_otp_sms to async
- Used httpx.AsyncClient for API calls
- Updated exception handling
- Modified app/api/v1/routes/auth_routes.py
- Updated SMS service call to use await
- Updated tests to support async execution
- Converted relevant tests to async
- Used AsyncMock where required
Technical Details
Root Cause:
- Blocking I/O due to synchronous requests usage in async FastAPI environment Solution:
- Migrated to httpx.AsyncClient
- Ensured proper async/await usage Architecture:
- No changes to structure (Router → Service → External API remains intact)
Type of Change
-
🐛 Bug fix (non-breaking change that fixes an issue) -
✨ New feature (non-breaking change that adds functionality) -
💥 Breaking change (fix or feature that would cause existing functionality to change) -
📝 Documentation update -
♻ ️ Refactor (no functional changes) -
⚡ Performance improvement -
🧪 Test update -
🔧 Configuration change -
🚨 Security fix -
🗑 ️ Deprecation (removing deprecated code) -->
Related Issues / References
Resolves #80
Screenshots or Screen Recordings
How to Validate Locally
Previous Behaviour
- SMS service used synchronous
requests - This blocked the FastAPI event loop
- Could degrade performance under load
Changes Made
- Replaced
requestswithhttpx.AsyncClient - Converted SMS sending function to async
- Updated route to use
await - Adjusted error handling to avoid crashes (return
Noneinstead of raising)
Testing Done
-
Unit tests added/updated -
API endpoint tests passing
Test Cases Covered:
| Scenario | Expected Result | Status |
|---|---|---|
|
|
||
|
|
||
|
|
Test Commands Run:
# Example: Run all tests
pytest
# Example: Run specific test file
pytest tests/test_api_v1/test_patient_routes.py -v
# Example: Run with coverage
pytest --cov=app
Code Quality Checklist
Code Standards
-
Code follows project conventions (naming, structure, formatting) -
No debug statements or commented-out code left (unless necessary and intended) -
No unused imports, variables, or functions -
No duplicate code (DRY principle followed) -
Type hints are properly defined (no Anyunless justified and no mypy type check errors) -
Ruff checks pass: ruff check . ruff format . --check
Python & FastAPI Best Practices
-
Functions follow single-responsibility principle -
Async/await used correctly (no blocking calls in async functions) -
Dependency injection used appropriately -
Pydantic models used for request/response validation -
SQLAlchemy queries are optimized (no N+1 queries) -
Error handling is comprehensive (try/except with proper logging)
API Design
-
RESTful conventions followed -
Proper HTTP status codes returned -
Input validation implemented -
Authentication/authorization enforced -
Role Base access control used for user restriction -
API documentation (docstrings) updated
Database & Migrations
-
Database migrations created (if schema changed) -
Database migrations version is pointing to the latest version (and version name follows project conventions) -
Migrations are reversible (migrations contain downgrade scripts) -
Indexes added for frequently queried fields -
No raw SQL queries (using SQLAlchemy ORM) -
Data integrity constraints maintained
Security
-
No sensitive data logged (passwords, tokens, PII) -
SQL injection prevention verified (ORM used) -
Input sanitization implemented -
Authentication tokens handled securely -
CORS settings appropriate -
Security scan passes: bandit -r app/
Error Handling
-
Errors are caught and handled gracefully -
User-friendly error messages returned -
Errors are logged appropriately -
HTTP error responses follow API standards
Documentation
-
README.md updated (if setup steps changed) -
.env.exampleupdated (if new env vars added) -
API documentation updated (docstrings, OpenAPI specs) -
CHANGELOG.md will be updated (if applicable) -
Code comments explain complex logic (not what, but why)
Known Limitations / Technical Debt
Additional Notes
MR Acceptance Checklist
Quality & Correctness
-
Code works as intended and solves the stated problem -
No bugs introduced (existing functionality not broken) -
Edge cases handled appropriately
Maintainability
-
Code is readable and well-organized -
Code is testable and well-tested -
Follows project patterns and conventions
Acceptance Review
-
Reviewed by at least 1 teammate -
Reviewed by product owner
-->
Screenshots or Screen Recordings
How to Validate Locally
Testing Done
-
Unit tests added/updated -
API endpoint tests passing
Test Cases Covered:
| Scenario | Expected Result | Status |
|---|---|---|
|
|
||
|
|
||
|
|
Test Commands Run:
# Example: Run all tests
pytest
# Example: Run specific test file
pytest tests/test_api_v1/test_patient_routes.py -v
# Example: Run with coverage
pytest --cov=app
Code Quality Checklist
Code Standards
-
Code follows project conventions (naming, structure, formatting) -
No debug statements or commented-out code left (unless necessary and intended) -
No unused imports, variables, or functions -
No duplicate code (DRY principle followed) -
Type hints are properly defined (no Anyunless justified and no mypy type check errors) -
Ruff checks pass: ruff check . ruff format . --check
Python & FastAPI Best Practices
-
Functions follow single-responsibility principle -
Async/await used correctly (no blocking calls in async functions) -
Dependency injection used appropriately -
Pydantic models used for request/response validation -
SQLAlchemy queries are optimized (no N+1 queries) -
Error handling is comprehensive (try/except with proper logging)
API Design
-
RESTful conventions followed -
Proper HTTP status codes returned -
Input validation implemented -
Authentication/authorization enforced -
Role Base access control used for user restriction -
API documentation (docstrings) updated
Database & Migrations
-
Database migrations created (if schema changed) -
Database migrations version is pointing to the latest version (and version name follows project conventions) -
Migrations are reversible (migrations contain downgrade scripts) -
Indexes added for frequently queried fields -
No raw SQL queries (using SQLAlchemy ORM) -
Data integrity constraints maintained
Security
-
No sensitive data logged (passwords, tokens, PII) -
SQL injection prevention verified (ORM used) -
Input sanitization implemented -
Authentication tokens handled securely -
CORS settings appropriate -
Security scan passes: bandit -r app/
Error Handling
-
Errors are caught and handled gracefully -
User-friendly error messages returned -
Errors are logged appropriately -
HTTP error responses follow API standards
Documentation
-
README.md updated (if setup steps changed) -
.env.exampleupdated (if new env vars added) -
API documentation updated (docstrings, OpenAPI specs) -
CHANGELOG.md will be updated (if applicable) -
Code comments explain complex logic (not what, but why)
Known Limitations / Technical Debt
Additional Notes
MR Acceptance Checklist
Quality & Correctness
-
Code works as intended and solves the stated problem -
No bugs introduced (existing functionality not broken) -
Edge cases handled appropriately
Maintainability
-
Code is readable and well-organized -
Code is testable and well-tested -
Follows project patterns and conventions
Acceptance Review
-
Reviewed by at least 1 teammate -
Reviewed by product owner