feat: updated dependencies including FastAPI last performance improvement#381
Merged
feat: updated dependencies including FastAPI last performance improvement#381
Conversation
Contributor
Reviewer's GuideUpdates core dependencies (notably FastAPI, aiosqlite, prometheus-client, pydantic-settings, sentry-sdk, fakeredis, uv) and tightens several URL-parsing helpers and robustness checks to be more precise and defensive, plus minor code-style and logging safety tweaks. Flow diagram for updated URL parsing helpers in players and roles modulesflowchart TD
subgraph RankDivisionParsing
A1["Input rank_url"] --> B1["Extract last path segment using rsplit('/', 1)"]
B1 --> C1["Take part before first '-' using split('-', 1)"]
C1 --> D1["Take part after last '_' using rsplit('_', 1)"]
D1 --> E1["Remove last 4 characters (e.g. '.png') and lower()"]
E1 --> F1["Construct CompetitiveDivision from normalized division name"]
end
subgraph EndorsementParsing
A2["Input frame_url"] --> B2["Extract last path segment using rsplit('/', 1)"]
B2 --> C2["Take part before first '-' using split('-', 1)"]
C2 --> D2["Convert to int"]
D2 -->|success| E2["Return endorsement level"]
D2 -->|ValueError| F2["Return 0 as fallback"]
end
subgraph RoleKeyParsing
A3["Input icon_url"] --> B3["Extract last path segment using rsplit('/', 1)"]
B3 --> C3["Take part before first '-' using split('-', 1)"]
C3 --> D3["Uppercase key"]
D3 --> E3["Map OFFENSE to CompetitiveRole.DAMAGE"]
D3 --> F3["Map TANK to CompetitiveRole.TANK"]
D3 --> G3["Otherwise CompetitiveRole.SUPPORT"]
end
subgraph TierParsing
A4["Input tier_url"] --> B4["Check if tier_url is None; if None return 0"]
B4 -->|not None| C4["tier_url.split('/')[-1]"]
C4 --> D4["Take part before first '-' using split('-', 1)"]
D4 --> E4["Take part after last '_' using split('_')[-1]"]
E4 --> F4["Convert to int"]
F4 -->|success| G4["Return tier value"]
F4 -->|IndexError or ValueError| H4["Return 0 as fallback"]
end
subgraph RoleIconHelper
A5["Input role icon URL"] --> B5["Extract last path segment using rsplit('/', 1)"]
B5 --> C5["Take part before first '.' using split('.', 1)"]
C5 --> D5["Lowercase key"]
D5 --> E5["Return role key name"]
end
Flow diagram for updated roles HTML parsing with safer icon handlingflowchart TD
A["parse_roles_html(html)"] --> B["Parse HTML document"]
B --> C["Locate role headers and role icon URLs list roles_icons"]
C --> D["For each role_index in headers"]
D --> E["Select icon candidate: roles_icons[role_index] or ''"]
E --> F["Call get_role_from_icon_url with non-None string"]
F --> G["get_role_from_icon_url uses rsplit and split to extract key and lower()"]
G --> H["Return role key"]
D --> I["Extract role name from header h3 and capitalize"]
D --> J["Extract role description div text"]
D --> K["Read raw icon URL from roles_icons[role_index]"]
H --> L["Build role dict: { key, name, icon, description }"]
I --> L
J --> L
K --> L
L --> M["Append role dict to roles list"]
M --> N["After loop, return roles list"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The changes from
except (IndexError, ValueError):/except (OSError, sqlite3.Error):toexcept IndexError, ValueError:andexcept OSError, sqlite3.Error:introduce invalid Python 3 syntax; these should remain as a single tuple in oneexceptclause or be split into two separateexceptblocks. - If the goal of the updated
rsplit(..., maxsplit=1)usages is to make URL parsing more robust, consider applying the same pattern inget_tier_from_icon, which still uses chainedsplit()calls that are more brittle than the revised helpers around it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The changes from `except (IndexError, ValueError):`/`except (OSError, sqlite3.Error):` to `except IndexError, ValueError:` and `except OSError, sqlite3.Error:` introduce invalid Python 3 syntax; these should remain as a single tuple in one `except` clause or be split into two separate `except` blocks.
- If the goal of the updated `rsplit(..., maxsplit=1)` usages is to make URL parsing more robust, consider applying the same pattern in `get_tier_from_icon`, which still uses chained `split()` calls that are more brittle than the revised helpers around it.
## Individual Comments
### Comment 1
<location> `app/players/helpers.py:146` </location>
<code_context>
if not blizzard_id:
return None
- except (IndexError, ValueError):
+ except IndexError, ValueError:
logger.warning(f"Failed to extract Blizzard ID from URL: {url}")
return None
</code_context>
<issue_to_address>
**issue (bug_risk):** The `except IndexError, ValueError` syntax is invalid in modern Python and will cause a `SyntaxError`.
In Python 3, multiple exceptions must be wrapped in a tuple. This should be `except (IndexError, ValueError):`; the current syntax raises a `SyntaxError` and will prevent the module from importing.
</issue_to_address>
### Comment 2
<location> `app/adapters/blizzard/parsers/utils.py:113` </location>
<code_context>
if not blizzard_id:
return None
- except (IndexError, ValueError):
+ except IndexError, ValueError:
logger.warning(f"Failed to extract Blizzard ID from URL: {url}")
return None
</code_context>
<issue_to_address>
**issue (bug_risk):** The updated exception handler uses Python 2 style syntax and will not parse in Python 3.
Please revert to `except (IndexError, ValueError):`; the current syntax is invalid in Python 3 and will prevent this module from importing.
</issue_to_address>
### Comment 3
<location> `app/adapters/storage/sqlite_storage.py:135` </location>
<code_context>
replacement = await self._create_connection()
await self._pool.put(replacement)
- except (OSError, sqlite3.Error):
+ except OSError, sqlite3.Error:
# Replacement failed — tombstone keeps the slot so the queue
# never blocks permanently; _acquire_connection will retry.
</code_context>
<issue_to_address>
**issue (bug_risk):** Catching multiple exceptions with commas is invalid syntax in Python 3.
Change this to `except (OSError, sqlite3.Error):`; the current form is invalid in Python 3 and will raise a `SyntaxError`, breaking the storage layer.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Update URL parsing helpers and logging safety while bumping core dependencies and tooling versions.
Bug Fixes:
Enhancements:
Build:
CI: