Address code review feedback
- Fix background_task scope issue (use global consistently) - Improve IMAP exception handling comment with specific examples - Move random import to module level with TODO comment - Add nosec B311 comment for mock data random usage All code review issues resolved. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
from datetime import datetime, timedelta
|
||||
from typing import Any, Dict, List, Optional
|
||||
import random # Used for mock data generation - TODO: Replace with actual historical data
|
||||
|
||||
from app.services.report_store import ReportStore
|
||||
from fastapi import APIRouter, HTTPException, Path, Query, status
|
||||
@@ -298,11 +299,9 @@ async def get_domain_reports(
|
||||
date = datetime.now() - timedelta(days=i)
|
||||
date_str = date.strftime("%Y-%m-%d")
|
||||
|
||||
# For Milestone 1, generate some mock data with variation
|
||||
# In future milestone, this will use actual historical data
|
||||
import random
|
||||
|
||||
compliance_rate = random.uniform(80, 100)
|
||||
# TODO: Replace with actual historical data in future milestone
|
||||
# For now, generate mock data with variation for demonstration purposes
|
||||
compliance_rate = random.uniform(80, 100) # nosec B311 - Mock data only
|
||||
|
||||
timeline.append(TimelinePoint(date=date_str, compliance_rate=round(compliance_rate, 1)))
|
||||
|
||||
|
||||
+2
-5
@@ -69,9 +69,6 @@ async def scheduled_imap_polling():
|
||||
|
||||
def create_app() -> FastAPI:
|
||||
"""Create and configure the FastAPI application"""
|
||||
# Task management for background jobs
|
||||
background_task = None
|
||||
|
||||
app = FastAPI(
|
||||
title=settings.PROJECT_NAME,
|
||||
openapi_url=f"{settings.API_V1_STR}/openapi.json",
|
||||
@@ -119,7 +116,7 @@ def create_app() -> FastAPI:
|
||||
@app.on_event("startup")
|
||||
async def startup_event():
|
||||
"""Initialize background tasks and security on application startup"""
|
||||
nonlocal background_task
|
||||
global background_task
|
||||
|
||||
# Generate and provide admin API key
|
||||
api_key = generate_api_key()
|
||||
@@ -151,7 +148,7 @@ def create_app() -> FastAPI:
|
||||
@app.on_event("shutdown")
|
||||
async def shutdown_event():
|
||||
"""Clean up background tasks on application shutdown"""
|
||||
nonlocal background_task # noqa: F824
|
||||
global background_task
|
||||
if background_task:
|
||||
logger.info("Cancelling IMAP polling background task")
|
||||
background_task.cancel()
|
||||
|
||||
@@ -87,7 +87,10 @@ class IMAPClient:
|
||||
available_mailboxes.append(mailbox_name)
|
||||
except Exception:
|
||||
# Silently skip mailboxes that can't be parsed
|
||||
# This is expected for some IMAP server responses
|
||||
# Some IMAP servers return non-standard list responses or
|
||||
# use different delimiters/encodings that don't follow RFC 3501
|
||||
# Common cases: special characters, non-UTF8 encodings, malformed responses
|
||||
# This is expected behavior and not a critical error
|
||||
pass # nosec B110
|
||||
|
||||
# Select inbox and get message count
|
||||
|
||||
Reference in New Issue
Block a user