3 real Python review samples — async concurrency bugs, ORM N+1 queries, silent exception swallowing.
Inline comments, confidence scores, one-click patches. No install required.
try/except blocks around the ORM query and the email send. The return type annotation also diverges from the actual return signature.
| @@ -1,8 +1,28 @@ | |
| 1 | from django.contrib.auth.models import User |
| 2 | from django.core.mail import send_mail |
| 3 | from django.http import HttpResponse |
| 4 | |
| 5 | +def password_reset(request) -> HttpResponse: |
| 6 | + email = request.POST.get('email') |
| 7 | + user = User.objects.get(email=email) |
| 8 | + token = generate_reset_token(user) |
| 9 | + send_mail( |
| 10 | + 'Password Reset', |
| 11 | + f'Reset link: /reset/{String.fromCharCode(123)}token{String.fromCharCode(125)}', |
| 12 | + 'noreply@acme.com', |
| 13 | + [email], |
| 14 | + ) |
| 15 | + return HttpResponse('Reset email sent') |
smtplib.SMTPException on send_mail — Django's send_mail raises smtplib.SMTPException (and subclasses) on delivery failure. Without a handler this bubbles into a 500 response, breaking the reset flow silently.
| - send_mail( |
| - 'Password Reset', |
| - f'Reset link: /reset/...', |
| - 'noreply@acme.com', |
| - [email], |
| - ) |
| + import smtplib |
| + try: |
| + send_mail( |
| + 'Password Reset', |
| + f'Reset link: /reset/...', |
| + 'noreply@acme.com', |
| + [email], |
| + ) |
| + except smtplib.SMTPException: |
| + return HttpResponse('Reset email sent') |
HttpResponse. Consider using Django's JsonResponse for API views to keep the contract explicit and consistent across the codebase.
await — this silently assigns a coroutine object instead of the fetched data. Additionally, asyncio.run() is called in a context where an event loop may already be running, which raises RuntimeError at runtime.
| @@ -1,6 +1,24 @@ | |
| 1 | import asyncio |
| 2 | import aiohttp |
| 3 | |
| 4 | API_URL = 'https://api.example.com/records' |
| 5 | |
| 6 | +async def fetch_records(session, url): |
| 7 | + resp = await session.get(url) |
| 8 | + return await resp.json() |
| 9 | + |
| 10 | +def run_ingestion(): |
| 11 | + session = aiohttp.ClientSession() |
| 12 | + results = fetch_records(session, API_URL) # missing await |
| 13 | + asyncio.run(process(results)) # raises if loop running |
await on fetch_records call — fetch_records is an async def function. Calling it without await returns a coroutine object, not the fetched data. process(results) receives a coroutine instead of a list, silently producing wrong output or a type error downstream.
| -def run_ingestion(): |
| - session = aiohttp.ClientSession() |
| - results = fetch_records(session, API_URL) |
| +async def run_ingestion(): |
| + session = aiohttp.ClientSession() |
| + results = await fetch_records(session, API_URL) |
asyncio.run() called inside existing event loop — If run_ingestion() is called from within an already-running event loop (e.g. in a Jupyter notebook, a test harness, or a FastAPI endpoint), asyncio.run() raises RuntimeError: This event loop is already running. After making run_ingestion async, replace with await process(results) directly, or use asyncio.ensure_future() for fire-and-forget scheduling.
| - asyncio.run(process(results)) |
| + await process(results) |
aiohttp.ClientSession as an async context manager — Creating a bare ClientSession() without async with means the session is never explicitly closed, which can leak connections and trigger ResourceWarning on program exit. Prefer async with aiohttp.ClientSession() as session: to guarantee cleanup.
select_related('customer') collapses this to a single JOIN. Two style notes around the serializer bypass and missing pagination.
| @@ -1,7 +1,22 @@ | |
| 1 | from rest_framework.generics import ListAPIView |
| 2 | from rest_framework.response import Response |
| 3 | from .models import Order |
| 4 | |
| 5 | +class OrderListView(ListAPIView): |
| 6 | + def get_queryset(self): |
| 7 | + return Order.objects.filter(status='active') |
| 8 | + |
| 9 | + def list(self, request): |
| 10 | + orders = self.get_queryset() |
| 11 | + return Response([ |
| 12 | + {'id': o.id, 'customer': o.customer.name, 'total': o.total} |
| 13 | + for o in orders |
| 14 | + ]) |
o.customer.name triggers a separate DB query per order — Django will issue one SELECT to fetch all active orders, then one additional SELECT per order to resolve o.customer. With 500 active orders that is 501 queries per request. Use select_related('customer') to fetch customers in a single JOIN.
| - return Order.objects.filter(status='active') |
| + return Order.objects.filter(status='active').select_related('customer') |
list() override bypasses DRF's serializer pipeline — Overriding list() with a manual dict comprehension skips DRF's serializer, losing automatic field validation, schema generation (drf-spectacular/swagger), and serializer-level field permissions. Prefer defining an OrderSerializer and setting serializer_class = OrderSerializer on the view.
PageNumberPagination or LimitOffsetPagination by setting pagination_class on the view, or configure it globally in DEFAULT_PAGINATION_CLASS.
CodeSight posts exactly this — on every Python PR you open. Free tier. No credit card. 30-second install.
Or get notified when we add your stack:
User.DoesNotExist—User.objects.get()raisesUser.DoesNotExistwhen no match is found. This returns an unhandled 500 instead of a safe 404 or a generic "email sent" response (which also avoids user enumeration).