fix: replace 3.10 union syntax to resolve pytest collection failures#54
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/worker.py (1)
2156-2166:⚠️ Potential issue | 🔴 CriticalCritical bug: Using deprecated
encrypt()function that always raises RuntimeError.Lines 2162-2163 call the synchronous
encrypt()function, which was deprecated (see lines 270-272) and intentionally raises a RuntimeError with the message: "encrypt() is deprecated — use await encrypt_aes() instead". This means_create_notificationwill fail every time it tries to create a notification.Since this function is already async, you should use the async
encrypt_aes()function instead.🔧 Proposed fix
try: enc = env.ENCRYPTION_KEY await env.DB.prepare( "INSERT INTO notifications (id, user_id, type, title, message, related_id)" " VALUES (?, ?, ?, ?, ?, ?)" ).bind(new_id(), user_id, type_, - encrypt(title, enc), encrypt(message, enc), + await encrypt_aes(title, enc), await encrypt_aes(message, enc), related_id).run() except Exception as exc: await capture_exception(exc, env=env, where="_create_notification")Note: This same issue exists in
api_list_notificationsat lines 2206-2207, wheredecrypt()is called instead ofawait decrypt_aes(). You'll want to fix that too:"title": decrypt(r.title or "", env.ENCRYPTION_KEY), "message": decrypt(r.message or "", env.ENCRYPTION_KEY),should be:
"title": await decrypt_aes(r.title or "", env.ENCRYPTION_KEY), "message": await decrypt_aes(r.message or "", env.ENCRYPTION_KEY),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 2156 - 2166, Replace the deprecated synchronous encrypt()/decrypt() calls with the async AES variants: in _create_notification use await encrypt_aes(enc, title) and await encrypt_aes(enc, message) (where enc = env.ENCRYPTION_KEY) before binding to the DB, and in api_list_notifications replace decrypt(...) with await decrypt_aes(enc, ...) when decrypting stored fields; keep the same exception handling (capture_exception) and ensure the async calls are awaited within the existing async functions (_create_notification and api_list_notifications) so the code no longer raises the intentional RuntimeError from encrypt()/decrypt().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Line 43: The review suggests modernizing type annotations by replacing
typing.Dict with the built-in dict (Python 3.9+); update the import line (remove
Dict) and replace occurrences of Dict in signatures/annotations across this
module (references: the import statement "from typing import Any, Dict,
Optional" and any function/class/type hints using "Dict") to use the built-in
"dict" type while keeping "Any" and "Optional" as needed so annotations remain
correct on Python 3.9.6.
---
Outside diff comments:
In `@src/worker.py`:
- Around line 2156-2166: Replace the deprecated synchronous encrypt()/decrypt()
calls with the async AES variants: in _create_notification use await
encrypt_aes(enc, title) and await encrypt_aes(enc, message) (where enc =
env.ENCRYPTION_KEY) before binding to the DB, and in api_list_notifications
replace decrypt(...) with await decrypt_aes(enc, ...) when decrypting stored
fields; keep the same exception handling (capture_exception) and ensure the
async calls are awaited within the existing async functions
(_create_notification and api_list_notifications) so the code no longer raises
the intentional RuntimeError from encrypt()/decrypt().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f7097ae-936e-4957-81ee-6de45eaab621
📒 Files selected for processing (1)
src/worker.py
2 participants
Description
This PR fixes a
TypeErrorduringpytesttest collection caused by using Python 3.10+ type union syntax (str | None) in a Python 3.9 environment. The codebase targets Python 3.9.6, which does not support the|union operator for type hints.Reverting this to
Optional[str]maintains the intended type safety while restoring compatibility.Changes Made
Optionalto typing imports inworker.pystr | NonewithOptional[str]in the_create_notificationfunction signature.All 273 tests now collect and execute successfully with zero breaking changes to functionality.
Summary
This PR resolves pytest test collection failures caused by using Python 3.10+ union type syntax (
str | None) in a codebase targeting Python 3.9.6.Changes
The type hint in the
_create_notificationhelper function insrc/worker.pyhas been updated:str | NonewithOptional[str]for therelated_idparameterOptionalto the typing importsImpact