Feat: Implement API Key Authentication and Standardize Logging#72
Feat: Implement API Key Authentication and Standardize Logging#72alebmorais wants to merge 2 commits intomainfrom
Conversation
This commit introduces two major improvements to the application: 1. **API Key Authentication:** All API endpoints are now protected by an API key. This is a critical security enhancement to prevent unauthorized access. The API key is configured via the `API_KEY` environment variable and must be sent in the `X-API-Key` header. 2. **Standardized Logging:** All `print` statements have been replaced with the `logging` module. A centralized logger is configured to write to `logs/app.log`, providing a structured and persistent record of application events and errors. This will significantly improve debugging and observability. In addition to these main changes, a duplicate `set_sense_mode` method was removed to improve code clarity.
Summary of ChangesHello @alebmorais, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's security posture by implementing API key authentication across all endpoints. Concurrently, it improves the application's maintainability and diagnostic capabilities by migrating from ad-hoc Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds API key authentication for FastAPI endpoints via an APIKeyHeader dependency and get_api_key validator; introduces API_KEY in .env.example. Replaces print/traceback calls with structured Python logging and standardizes error handling across endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI
participant get_api_key
participant Endpoint
participant Logger
Client->>FastAPI: HTTP request (Authorization / x-api-key)
FastAPI->>get_api_key: Depends(api_key_header) -> validate against API_KEY
alt valid
get_api_key->>FastAPI: returns key
FastAPI->>Endpoint: call handler
Endpoint->>Logger: info/debug logs (operation)
Endpoint->>Client: 200 OK / payload
else invalid or missing
get_api_key->>Logger: warning/error (unauthorized)
get_api_key->>Client: 401 Unauthorized (HTTPException)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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 significantly improves the application's security and observability by introducing API key authentication and standardizing on the logging module. The changes are well-implemented and consistent across the codebase. I have identified a few important issues: a security vulnerability (timing attack) in the API key comparison, a missing check for the API key at startup, and an unprotected WebSocket endpoint that contradicts the goal of securing all endpoints. Details and suggestions are provided in the specific comments.
| logging.basicConfig( | ||
| level=logging.INFO, | ||
| format="%(asctime)s - %(levelname)s - %(message)s", | ||
| handlers=[ | ||
| logging.FileHandler(LOG_DIR / "app.log"), | ||
| logging.StreamHandler(), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Using logging.basicConfig() configures the root logger. This can sometimes cause issues if third-party libraries also try to configure logging, as it's a global configuration. A more robust approach is to create and configure a specific logger for your application, like logger = logging.getLogger("pi_productivity"). This isolates your application's logging configuration and prevents it from affecting or being affected by other libraries. While basicConfig might be acceptable for a simple, standalone application, using a named logger is a better practice for maintainability.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @app.post("/sense/mode", response_class=JSONResponse) | ||
| async def set_sense_mode_endpoint(mode_name: str = Form(...)): | ||
| async def set_sense_mode_endpoint( | ||
| mode_name: str = Form(...), api_key: str = Depends(get_api_key) | ||
| ): | ||
| new_mode = await run_in_threadpool(sense.set_sense_mode, mode_name) | ||
| return JSONResponse({"status": "success", "mode": new_mode}) | ||
|
|
||
| @app.post("/ocr", response_class=JSONResponse) | ||
| async def run_ocr_endpoint(): | ||
| async def run_ocr_endpoint(api_key: str = Depends(get_api_key)): | ||
| try: | ||
| img_path, txt_path, text = await run_in_threadpool(sense.run_ocr_once) | ||
| return JSONResponse({"status": "success", "image_path": img_path, "text_path": txt_path, "text": text}) | ||
| except Exception as e: | ||
| # 1. (SEGURO) Loga o erro completo no seu terminal/console | ||
| # Assim VOCÊ pode ver o que deu errado. | ||
| print("--- ERRO NO ENDPOINT DE OCR ---") | ||
| traceback.print_exc() | ||
| print("-------------------------------") | ||
| logging.error("Error in OCR endpoint", exc_info=True) | ||
|
|
||
| # 2. (SEGURO) Envia uma mensagem genérica para o usuário | ||
| # O usuário/invasor não vê nenhuma informação sensível. | ||
| return JSONResponse( | ||
| {"status": "error", "message": "Ocorreu um erro interno ao processar a imagem."}, | ||
| status_code=500 | ||
| {"status": "error", "message": "Ocorreu um erro interno ao processar a imagem."}, | ||
| status_code=500, | ||
| ) | ||
| @app.get("/camera.jpg") | ||
| async def camera_jpeg(): | ||
| async def camera_jpeg(api_key: str = Depends(get_api_key)): | ||
| frame = await run_in_threadpool(sense.read_jpeg) | ||
| data = frame or b'' # Return empty bytes if no frame | ||
| data = frame or b'' # Return empty bytes if no frame | ||
| return StreamingResponse(io.BytesIO(data), media_type="image/jpeg") | ||
|
|
||
| @app.get("/api/week-calendar", response_class=JSONResponse) | ||
| async def get_week_calendar(): | ||
| async def get_week_calendar(api_key: str = Depends(get_api_key)): |
There was a problem hiding this comment.
The new Depends(get_api_key) requirement on /sense/mode, /ocr, /camera.jpg, and /api/week-calendar forces every request to supply an X-API-Key header. None of the existing browser calls add this header (static/app.js posts to /sense/mode and /ocr and fetches /api/week-calendar, and the <img> tag for /camera.jpg cannot send custom headers). After this change the web UI cannot set modes, trigger OCR, load the week calendar, or display the camera feed because every call now returns 401/403. Either keep these endpoints anonymous or update the front-end to attach the key (and provide a workable approach for the camera image which currently cannot be authenticated).
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.py (1)
484-492: WebSocket is unauthenticated; add API key check beforeaccept()/ws currently accepts all clients, bypassing the new API key layer. Enforce auth via header or query param and close with policy violation when invalid. Security-critical.
-@app.websocket("/ws") -async def websocket_endpoint(ws: WebSocket): - await bus.add(ws) +@app.websocket("/ws") +async def websocket_endpoint(ws: WebSocket): + # Authenticate via header or query param before accepting + key = ws.headers.get("x-api-key") or ws.query_params.get("api_key") + if not API_KEY or key != API_KEY: + # 1008 = Policy Violation + await ws.close(code=1008) + return + await bus.add(ws)
🧹 Nitpick comments (6)
.env.example (1)
7-8: API_KEY formatting + guidanceTo satisfy dotenv-linter and reduce friction, drop quotes and add a quick generation tip.
-# A secret key to protect your API endpoints -API_KEY="your_secret_api_key" +# A secret key to protect your API endpoints +# Generate with: python -c "import secrets; print(secrets.token_urlsafe(32))" +API_KEY=your_secret_api_keymain.py (5)
461-463: Drop unused exception variableSilences Ruff F841 without changing behavior.
- except Exception as e: - logging.error("Error in OCR endpoint", exc_info=True) + except Exception: + logging.error("Error in OCR endpoint", exc_info=True)
47-54: Harden logging: rotation, reload-safety, and encodingUse RotatingFileHandler to prevent unbounded growth and force=True to avoid duplicate handlers under uvicorn reload.
+from logging.handlers import RotatingFileHandler @@ -logging.basicConfig( +logging.basicConfig( level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s", - handlers=[ - logging.FileHandler(LOG_DIR / "app.log"), - logging.StreamHandler(), - ], -) + handlers=[ + RotatingFileHandler(LOG_DIR / "app.log", maxBytes=10_000_000, backupCount=5, encoding="utf-8"), + logging.StreamHandler(), + ], + force=True, +)Also applies to: 9-9
126-137: Optional: TRY300 — move success log intoelseblockMinor clarity per Ruff TRY300.
- try: - cam = Picamera2() - # Reduzir a resolução pode acelerar a captura e o processamento - config = cam.create_still_configuration(main={"size": (1024, 576)}) - cam.configure(config) - cam.start() - time.sleep(1) # Allow camera to warm up - logging.info("Camera initialized.") - return cam - except Exception as e: - logging.error(f"Error initializing camera: {e}", exc_info=True) - return None + try: + cam = Picamera2() + config = cam.create_still_configuration(main={"size": (1024, 576)}) + cam.configure(config) + cam.start() + time.sleep(1) # Allow camera to warm up + except Exception as e: + logging.error(f"Error initializing camera: {e}", exc_info=True) + return None + else: + logging.info("Camera initialized.") + return cam
533-533: Keep a handle to background task and clean up on shutdownPrevents orphaned tasks (RUF006) and ensures graceful shutdown.
- asyncio.create_task(broadcast_loop()) + app.state.broadcast_task = asyncio.create_task(broadcast_loop())Add a shutdown hook (new code outside the shown hunk):
@app.on_event("shutdown") async def on_shutdown(): task = getattr(app.state, "broadcast_task", None) if task: task.cancel() with suppress(asyncio.CancelledError): await task
251-255: Convert remaining print statements to logging for consistency across modulesThe logging improvements in main.py are good. Verification confirms print statements still exist in: motion_client.py, sense_mode.py, epaper.py, and analyze_productivity.py (16 total). These should be converted to logging to maintain the structured logging pattern established in main.py.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)main.py(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
main.py (4)
sense_mode.py (4)
is_running(133-135)stop(128-131)clear(36-38)start(120-126)motion_client.py (3)
list_all_tasks_simple(106-142)post(45-50)get(32-43)task_database.py (2)
upsert_motion_tasks(71-98)fetch_items_for_display(189-248)epaper.py (1)
render_list(66-91)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Ruff (0.14.1)
main.py
134-134: Consider moving this statement to an else block
(TRY300)
451-451: Unused function argument: api_key
(ARG001)
457-457: Unused function argument: api_key
(ARG001)
461-461: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
471-471: Unused function argument: api_key
(ARG001)
477-477: Unused function argument: api_key
(ARG001)
533-533: Store a reference to the return value of asyncio.create_task
(RUF006)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
main.py (1)
441-448: Confirm intent: should the HTML UI (/) also require API key?If yes, add
dependencies=[Depends(get_api_key)]to the route or set a global dependency on the app. Note: browsers can’t send custom headers on initial page load without a proxy or query parameter flow.
| OCR_INTERVAL = int(os.getenv("OCR_INTERVAL", "600")) | ||
| API_KEY = os.getenv("API_KEY") | ||
|
|
||
| api_key_header = APIKeyHeader(name="X-API-Key") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consistent 401 handling, constant-time compare, and auth header hint
Make missing/wrong key paths consistent and safer: disable auto_error, use constant-time compare, and return WWW-Authenticate for clients.
+import hmac
@@
-api_key_header = APIKeyHeader(name="X-API-Key")
+api_key_header = APIKeyHeader(name="X-API-Key", auto_error=False)
@@
-async def get_api_key(api_key: str = Depends(api_key_header)):
- if not API_KEY or api_key != API_KEY:
+async def get_api_key(api_key: str = Depends(api_key_header)):
+ if not API_KEY or not api_key or not hmac.compare_digest(api_key, API_KEY):
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Invalid or missing API Key",
+ headers={"WWW-Authenticate": "APIKey"},
)
return api_keyAlso applies to: 72-79
🤖 Prompt for AI Agents
In main.py around line 69 (and also lines 72-79), the APIKeyHeader is created
and key validation currently allows differing behavior for missing vs invalid
keys and uses a non-constant comparison; change the APIKeyHeader to disable
automatic errors (APIKeyHeader(..., auto_error=False)), then in the dependency
explicitly check if the header is absent or does not match the expected key
using secrets.compare_digest for a constant-time comparison, and for both
missing and wrong keys raise the same HTTPException with status_code=401 and
headers={"WWW-Authenticate":"API-Key"} to provide a consistent 401 response and
authentication hint to clients.
| @app.post("/sense/mode", response_class=JSONResponse) | ||
| async def set_sense_mode_endpoint(mode_name: str = Form(...)): | ||
| async def set_sense_mode_endpoint( | ||
| mode_name: str = Form(...), api_key: str = Depends(get_api_key) | ||
| ): | ||
| new_mode = await run_in_threadpool(sense.set_sense_mode, mode_name) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused api_key params; use decorator-level dependency
Eliminates ARG001 warnings and centralizes auth per-route.
-@app.post("/sense/mode", response_class=JSONResponse)
-async def set_sense_mode_endpoint(
- mode_name: str = Form(...), api_key: str = Depends(get_api_key)
-):
+@app.post("/sense/mode", response_class=JSONResponse, dependencies=[Depends(get_api_key)])
+async def set_sense_mode_endpoint(mode_name: str = Form(...)):
@@
-@app.post("/ocr", response_class=JSONResponse)
-async def run_ocr_endpoint(api_key: str = Depends(get_api_key)):
+@app.post("/ocr", response_class=JSONResponse, dependencies=[Depends(get_api_key)])
+async def run_ocr_endpoint():
@@
-@app.get("/camera.jpg")
-async def camera_jpeg(api_key: str = Depends(get_api_key)):
+@app.get("/camera.jpg", dependencies=[Depends(get_api_key)])
+async def camera_jpeg():
@@
-@app.get("/api/week-calendar", response_class=JSONResponse)
-async def get_week_calendar(api_key: str = Depends(get_api_key)):
+@app.get("/api/week-calendar", response_class=JSONResponse, dependencies=[Depends(get_api_key)])
+async def get_week_calendar():Also applies to: 456-469, 471-475, 477-483
🧰 Tools
🪛 Ruff (0.14.1)
451-451: Unused function argument: api_key
(ARG001)
🤖 Prompt for AI Agents
In main.py around lines 449-453 (and similarly at 456-469, 471-475, 477-483),
the endpoint functions declare an unused api_key parameter via
Depends(get_api_key); remove that unused parameter from the function signatures
and move the dependency into the route decorator (e.g., app.post(...,
dependencies=[Depends(get_api_key)]) or app.get(...,
dependencies=[Depends(get_api_key)]) as appropriate) so authentication is
applied at the decorator level and ARG001 warnings are eliminated.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.py (1)
484-498: Based on my verification of the codebase, the review comment is accurate and confirms a real authentication gap. Here are the key findings:Verification Results:
WebSocket endpoint is unauthenticated: The
/wsendpoint (lines 484-492) has no API key validation or any other authentication mechanism. It accepts all connections.Build payload includes task data:
build_status_payload()(lines 432-438) returns:
tasks: fromsense.db.fetch_items_for_display(limit=20)— task titles, subtitles, due dates, and statuses- Plus mode, sensor readings, and timestamp
API endpoint is protected: The
/api/week-calendarendpoint (lines 476-482) requires valid API key authentication viaapi_key: str = Depends(get_api_key)which validates theX-API-Keyheader.Broadcast exposes task data: The
broadcast_loop()(lines 494-498) publishes this payload to all connected WebSocket clients every 2 seconds without any authentication checks.Data is the same source: Both
fetch_items_for_display()andfetch_week_calendar()query the same task database with the same filters (non-completed tasks for the current week).
WebSocket
/wsremains unauthenticated while streaming protected task dataThe
/wsendpoint broadcastsbuild_status_payload()every 2 seconds, which includes task data (titles, subtitles, due dates, statuses) fromsense.db.fetch_items_for_display(). This same task data is protected by API key on the/api/week-calendarendpoint. An unauthenticated client can connect to/wsand receive unrestricted access to the same sensitive information.To align security across endpoints:
- Add lightweight auth check on the WebSocket handshake (e.g., query param
?api_key=...validated viaget_api_keylogic), or- Explicitly document that
/wsis intentionally public and task data is non-sensitive.Currently this represents a security inconsistency where HTTP endpoints require authentication but WebSocket streaming does not.
♻️ Duplicate comments (3)
main.py (3)
1-9: Fix undefinedsecretsimport and hardenget_api_keyhandling
get_api_keyusessecrets.compare_digestbutsecretsisn’t imported, which will raise aNameErroron the first authenticated request. Also, missing headers still go through FastAPI’s default 403 path rather than your custom 401 logic.I suggest:
- Add the missing import.
- Disable
APIKeyHeader’sauto_errorand allowNonein the dependency.- Normalize both missing and invalid keys to the same 401 response, and optionally add a
WWW-Authenticatehint.+import secrets @@ -API_KEY = os.getenv("API_KEY") - -api_key_header = APIKeyHeader(name="X-API-Key") +API_KEY = os.getenv("API_KEY") + +api_key_header = APIKeyHeader(name="X-API-Key", auto_error=False) @@ -async def get_api_key(api_key: str = Depends(api_key_header)): - if not API_KEY or not secrets.compare_digest(api_key, API_KEY): +async def get_api_key(api_key: str | None = Depends(api_key_header)): + if not API_KEY or not api_key or not secrets.compare_digest(api_key, API_KEY): raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid or missing API Key", + detail="Invalid or missing API Key", + headers={"WWW-Authenticate": "API-Key"}, ) return api_keyThis resolves F821 and gives you consistent, constant‑time API key validation.
Also applies to: 15-24, 28-28, 67-79
61-68: Consider failing fast whenAPI_KEYis missingRight now the app happily starts with
API_KEYunset, but all protected endpoints will then return 401, which can be confusing in deployment.Consider explicitly checking for
API_KEYduring startup and aborting if it’s missing, so misconfiguration is caught immediately:@app.on_event("startup") async def on_startup(): + if not API_KEY: + raise RuntimeError( + "API_KEY environment variable not set. The application cannot start without it." + ) # Instantiate the main application object here so hardware initializationThis aligns the runtime behavior with the “API key required” contract and surfaces config errors early.
Also applies to: 500-520
449-483: Clean up unusedapi_keyparams and OCR exception variable; verify client compatibilityTwo separate points in this block:
- Unused
api_keyparameters (ARG001) and cleaner auth wiringAll four endpoints declare
api_key: str = Depends(get_api_key)but never use the value, which Ruff flags as ARG001. A cleaner pattern is to move the dependency to the decorator and drop the unused parameter entirely:-@app.post("/sense/mode", response_class=JSONResponse) -async def set_sense_mode_endpoint( - mode_name: str = Form(...), api_key: str = Depends(get_api_key) -): +@app.post( + "/sense/mode", + response_class=JSONResponse, + dependencies=[Depends(get_api_key)], +) +async def set_sense_mode_endpoint(mode_name: str = Form(...)): @@ -@app.post("/ocr", response_class=JSONResponse) -async def run_ocr_endpoint(api_key: str = Depends(get_api_key)): +@app.post( + "/ocr", + response_class=JSONResponse, + dependencies=[Depends(get_api_key)], +) +async def run_ocr_endpoint(): @@ - except Exception as e: - logging.error("Error in OCR endpoint", exc_info=True) + except Exception: + logging.error("Error in OCR endpoint", exc_info=True) @@ -@app.get("/camera.jpg") -async def camera_jpeg(api_key: str = Depends(get_api_key)): +@app.get("/camera.jpg", dependencies=[Depends(get_api_key)]) +async def camera_jpeg(): @@ -@app.get("/api/week-calendar", response_class=JSONResponse) -async def get_week_calendar(api_key: str = Depends(get_api_key)): +@app.get( + "/api/week-calendar", + response_class=JSONResponse, + dependencies=[Depends(get_api_key)], +) +async def get_week_calendar():This removes the lint errors (including the unused
e) and centralizes auth per route.
- Confirm browser/front‑end clients still work with the new auth
Requiring
X-API-Keyon/sense/mode,/ocr,/camera.jpg, and/api/week-calendarmeans:
- Existing JS calls (e.g., from
static/app.js) must now attach the header.- The
<img src="/camera.jpg">in the template cannot send custom headers, so the camera stream will 401/403 unless you’ve reworked that flow (e.g., via a proxied endpoint, signed URL, or JS blob URL).Please double‑check that the front‑end has been updated accordingly and that all UI flows (mode switching, OCR trigger, week calendar, camera image) still function under the new auth model.
🧹 Nitpick comments (2)
main.py (2)
122-137: Optional: usetry/except/elsefor clearer camera init flow (TRY300)The camera init
tryblock mixes the happy path and error handling, which Ruff flags (TRY300). You can make success vs failure clearer by usingelse:try: cam = Picamera2() # Reduzir a resolução pode acelerar a captura e o processamento config = cam.create_still_configuration(main={"size": (1024, 576)}) cam.configure(config) cam.start() time.sleep(1) # Allow camera to warm up - logging.info("Camera initialized.") - return cam - except Exception as e: - logging.error(f"Error initializing camera: {e}", exc_info=True) - return None + except Exception as e: + logging.error("Error initializing camera", exc_info=True) + return None + else: + logging.info("Camera initialized.") + return camBehavior stays the same, but the control flow is cleaner and linter‑friendly.
533-534: Store thebroadcast_looptask reference (RUF006) for observability and lifecycle control
asyncio.create_task(broadcast_loop())is fire‑and‑forget; Ruff flags this (RUF006), and any exceptions in the task may become “never retrieved” warnings.You can keep a handle on the task (and optionally cancel it on shutdown) by storing it on the app:
- asyncio.create_task(broadcast_loop()) + app.state.broadcast_task = asyncio.create_task(broadcast_loop())If you later add a shutdown event, you can cancel
app.state.broadcast_taskcleanly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main.py(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
main.py (3)
sense_mode.py (4)
is_running(133-135)stop(128-131)clear(36-38)start(120-126)motion_client.py (4)
list_all_tasks_simple(106-142)post(45-50)get(32-43)create_task(153-159)task_database.py (2)
upsert_motion_tasks(71-98)fetch_items_for_display(189-248)
🪛 GitHub Check: Codacy Static Code Analysis
main.py
[warning] 73-73: main.py#L73
undefined name 'secrets' (F821)
🪛 Ruff (0.14.5)
main.py
73-73: Undefined name secrets
(F821)
134-134: Consider moving this statement to an else block
(TRY300)
451-451: Unused function argument: api_key
(ARG001)
457-457: Unused function argument: api_key
(ARG001)
461-461: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
471-471: Unused function argument: api_key
(ARG001)
477-477: Unused function argument: api_key
(ARG001)
533-533: Store a reference to the return value of asyncio.create_task
(RUF006)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
main.py (1)
165-167: Logging refactor looks solidThe move from
loggingwithexc_info=True, centralized handlers, and contextual messages across motion sync, joystick actions, OCR errors, startup, and hardware loops is consistent and appropriate for debugging on-device behavior. No functional issues spotted here.Also applies to: 229-230, 251-263, 268-268, 317-320, 337-355, 358-361, 374-379, 462-463, 467-468, 510-512, 523-528, 535-537
1 participant
This commit introduces two major improvements to the application:
API Key Authentication: All API endpoints are now protected by an API key. This is a critical security enhancement to prevent unauthorized access. The API key is configured via the
API_KEYenvironment variable and must be sent in theX-API-Keyheader.Standardized Logging: All
printstatements have been replaced with theloggingmodule. A centralized logger is configured to write tologs/app.log, providing a structured and persistent record of application events and errors. This will significantly improve debugging and observability.In addition to these main changes, a duplicate
set_sense_modemethod was removed to improve code clarity.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.