perf: replace blocking requests.get with aiohttp in get_player_headshot (#100) #166

Claude · 2026-04-10T16:01:29Z

Claude commented

2026-04-10 16:01:29 +00:00

Closes #100

Summary

Replaced the synchronous requests.get() call in get_player_headshot() with an async aiohttp.ClientSession request, matching the pattern already used in the adjacent get_player_photo() function in the same file.

Changes

helpers/main.py: get_player_headshot() — replaced requests.get(req_url, timeout=2).text with aiohttp.ClientSession async context manager using aiohttp.ClientTimeout(total=2). Timeout value preserved (2s). aiohttp was already imported at the top of the file.

Before

resp = requests.get(req_url, timeout=2).text
soup = BeautifulSoup(resp, "html.parser")

After

async with aiohttp.ClientSession() as session:
    async with session.get(
        req_url, timeout=aiohttp.ClientTimeout(total=2)
    ) as resp:
        text = await resp.text()
soup = BeautifulSoup(text, "html.parser")

Impact

Event loop no longer blocks on headshot cache misses. The bare except: pass fallback is preserved — any network error still falls through to get_player_photo().

Test Results

No test suite for this function. Ruff lint passed on commit.

Closes #100 ## Summary Replaced the synchronous `requests.get()` call in `get_player_headshot()` with an async `aiohttp.ClientSession` request, matching the pattern already used in the adjacent `get_player_photo()` function in the same file. ## Changes - `helpers/main.py`: `get_player_headshot()` — replaced `requests.get(req_url, timeout=2).text` with `aiohttp.ClientSession` async context manager using `aiohttp.ClientTimeout(total=2)`. Timeout value preserved (2s). `aiohttp` was already imported at the top of the file. ## Before ```python resp = requests.get(req_url, timeout=2).text soup = BeautifulSoup(resp, "html.parser") ``` ## After ```python async with aiohttp.ClientSession() as session: async with session.get( req_url, timeout=aiohttp.ClientTimeout(total=2) ) as resp: text = await resp.text() soup = BeautifulSoup(text, "html.parser") ``` ## Impact Event loop no longer blocks on headshot cache misses. The bare `except: pass` fallback is preserved — any network error still falls through to `get_player_photo()`. ## Test Results No test suite for this function. Ruff lint passed on commit.

Claude added 1 commit 2026-04-10 16:01:31 +00:00

perf: replace blocking requests.get with aiohttp in get_player_headshot (#100 )

Ruff Lint / lint (pull_request) Successful in 16s

Details

d957bbe7ab

Closes #100

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-04-10 16:01:36 +00:00

perf: replace blocking requests.get with aiohttp in get_player_headshot #100

Claude added the

ai-reviewing

label 2026-04-10 16:50:56 +00:00

Claude reviewed 2026-04-10 16:52:44 +00:00

Claude left a comment

AI Code Review

Files Reviewed

helpers/main.py (modified)

Findings

Correctness

No issues found.

text is assigned inside the async with blocks and consumed outside them — correct. Python has no block scoping so text is accessible after the context managers exit normally. If aiohttp raises before text is assigned, the exception propagates to except: pass and we never reach soup = BeautifulSoup(text, ...).
soup is placed outside the inner async with blocks — response connection is released before BeautifulSoup parses the body. Correct aiohttp usage: resp.text() reads the full body eagerly, so parsing safely happens after the context managers close.
aiohttp.ClientTimeout(total=2) matches the original timeout=2 semantics (total wall-clock timeout).
aiohttp confirmed imported at line 8. requests is not directly imported in helpers/main.py (arrives via star import from random_content); removing the only direct usage here is safe.
Bare except: pass fallback to get_player_photo() preserved unchanged.
Implementation matches the pattern of the adjacent get_player_photo() function exactly.

Security

No issues found.

Style & Conventions

Matches existing get_player_photo() pattern in the same file.

Suggestions

None.

Verdict: APPROVED

Clean async conversion. Correct context manager usage, error handling preserved, pattern-consistent with adjacent function. No correctness, security, or convention issues. (Posted as COMMENT — Gitea blocks self-approval.)

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `helpers/main.py` (modified) ### Findings #### Correctness No issues found. - `text` is assigned inside the `async with` blocks and consumed outside them — correct. Python has no block scoping so `text` is accessible after the context managers exit normally. If aiohttp raises before `text` is assigned, the exception propagates to `except: pass` and we never reach `soup = BeautifulSoup(text, ...)`. - `soup` is placed outside the inner `async with` blocks — response connection is released before BeautifulSoup parses the body. Correct aiohttp usage: `resp.text()` reads the full body eagerly, so parsing safely happens after the context managers close. - `aiohttp.ClientTimeout(total=2)` matches the original `timeout=2` semantics (total wall-clock timeout). - `aiohttp` confirmed imported at line 8. `requests` is not directly imported in `helpers/main.py` (arrives via star import from `random_content`); removing the only direct usage here is safe. - Bare `except: pass` fallback to `get_player_photo()` preserved unchanged. - Implementation matches the pattern of the adjacent `get_player_photo()` function exactly. #### Security No issues found. #### Style & Conventions Matches existing `get_player_photo()` pattern in the same file. #### Suggestions None. ### Verdict: APPROVED Clean async conversion. Correct context manager usage, error handling preserved, pattern-consistent with adjacent function. No correctness, security, or convention issues. (Posted as COMMENT — Gitea blocks self-approval.) --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-10 16:52:50 +00:00

Claude removed the

ai-reviewed

label 2026-04-11 19:23:45 +00:00

claude-reviewer added the

ai-reviewing

label 2026-04-11 21:21:19 +00:00

claude-reviewer approved these changes 2026-04-11 21:22:45 +00:00

claude-reviewer left a comment

AI Code Review

Files Reviewed

helpers/main.py (modified — get_player_headshot() only)

Findings

Correctness

No issues found.

text is assigned inside the innermost context manager. soup = BeautifulSoup(text, "html.parser") is correctly placed outside the async with blocks but still inside the try: — text is a fully-populated string at that point. No UnboundLocalError risk.
Timeout semantics preserved: requests.get(timeout=2) and aiohttp.ClientTimeout(total=2) are both total-timeout-2s.
Bare except: pass unchanged — asyncio.TimeoutError and aiohttp.ClientError are both caught and fall through to get_player_photo().
return await get_player_photo(player) fallback is outside the try/except (unchanged), so it still fires on both exception and no-headshot-found paths.
aiohttp already imported at line 8. No new imports needed.
requests is not directly imported in helpers/main.py (comes in transitively via from api_calls import *). The removal of the only requests.get() call leaves no dead direct import in this file.

Security

No issues found. URL is constructed from DB-sourced bbref_id/p_name — not user-supplied input. No change in attack surface.

Style & Conventions

Pattern matches get_player_photo() in the same file exactly (double async with aiohttp.ClientSession()). Clean.

Suggestions

No tests cover get_player_headshot() — acknowledged in PR. A smoke-test mocking aiohttp.ClientSession would guard against future regressions, but not a blocker.

Verdict: APPROVED

Functionally equivalent replacement. Async pattern is correct, timeout preserved, error handling unchanged. No correctness or security issues.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `helpers/main.py` (modified — `get_player_headshot()` only) ### Findings #### Correctness No issues found. - `text` is assigned inside the innermost context manager. `soup = BeautifulSoup(text, "html.parser")` is correctly placed outside the `async with` blocks but still inside the `try:` — `text` is a fully-populated string at that point. No `UnboundLocalError` risk. - Timeout semantics preserved: `requests.get(timeout=2)` and `aiohttp.ClientTimeout(total=2)` are both total-timeout-2s. - Bare `except: pass` unchanged — `asyncio.TimeoutError` and `aiohttp.ClientError` are both caught and fall through to `get_player_photo()`. - `return await get_player_photo(player)` fallback is outside the try/except (unchanged), so it still fires on both exception and no-headshot-found paths. - `aiohttp` already imported at line 8. No new imports needed. - `requests` is not directly imported in `helpers/main.py` (comes in transitively via `from api_calls import *`). The removal of the only `requests.get()` call leaves no dead direct import in this file. #### Security No issues found. URL is constructed from DB-sourced `bbref_id`/`p_name` — not user-supplied input. No change in attack surface. #### Style & Conventions Pattern matches `get_player_photo()` in the same file exactly (double `async with aiohttp.ClientSession()`). Clean. #### Suggestions - No tests cover `get_player_headshot()` — acknowledged in PR. A smoke-test mocking `aiohttp.ClientSession` would guard against future regressions, but not a blocker. ### Verdict: APPROVED Functionally equivalent replacement. Async pattern is correct, timeout preserved, error handling unchanged. No correctness or security issues. --- *Automated review by Claude PR Reviewer*

claude-reviewer added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-11 21:23:13 +00:00

cal force-pushed issue/100-perf-replace-blocking-requests-get-with-aiohttp-in from d957bbe7ab to 25551130e9

2026-04-12 14:52:14 +00:00

Compare

cal merged commit 61d61b9348 into main

2026-04-12 14:53:14 +00:00

cal deleted branch issue/100-perf-replace-blocking-requests-get-with-aiohttp-in

2026-04-12 14:53:14 +00:00