Merged
Conversation
Add optional cookies parameter to fetch_html and fetch_html_with_retry functions for both sync and async variants. Cookies are passed via temporary clients to avoid deprecation warnings and global client pollution. Update documentation to include team logos feature (logo_url_light, logo_url_dark) and remove version pinning from docs requirements.
Update teams info documentation to reflect new logo_url_light and logo_url_dark fields, with backward compatibility note for existing logo_url field defaulting to light mode logo.
Update version strings across all configuration files, documentation, and package metadata to reflect the new release. Files updated include pyproject.toml, __init__.py, docs conf, the documentation homepage, and uv.lock.
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Version Updates pyproject.toml, src/vlrdevapi/__init__.py, docs/source/conf.py, betterdocs/app/(home)/page.tsx |
Incremented version string from 1.6.1 to 1.6.2 across package metadata, source code, and UI components. |
Fetch Functions Enhancement src/vlrdevapi/fetcher.py |
Added optional cookies: dict[str, str] | None parameter to fetch_html_with_retry, fetch_html_with_retry_async, fetch_html, and fetch_html_async. When cookies are provided, functions create temporary clients with those cookies and bypass caching; otherwise they use existing pooled global clients. |
Teams Info API src/vlrdevapi/teams/info.py, src/vlrdevapi/teams/models.py |
Extended info() function to fetch both light (cookies={"dm": "0"}) and dark (cookies={"dm": "1"}) mode versions of team pages, extracting logo_url_light and logo_url_dark from respective DOMs. Added both fields to TeamInfo dataclass; logo_url now defaults to light mode for backward compatibility. |
Documentation Updates betterdocs/content/docs/api/teams/info.mdx, docs/source/api/teams.rst |
Updated API schema documentation to describe logo_url default behavior and added descriptions for new logo_url_light and logo_url_dark fields. Added "Get Team Logos" example section demonstrating usage of all three logo URL fields. |
Test Updates tests/lib/conftest.py, tests/lib/test_teams.py |
Enhanced mock_fetch_html to accept cookies parameter and return dark/light HTML variants based on cookies["dm"] value. Extended existing tests to verify new logo fields and added new TestTeamsDarkLightLogos class with assertions for light/dark logo URLs and backward compatibility. |
Dependency Configuration docs/requirements.txt |
Removed minimum version constraints from sphinx, sphinx-rtd-theme, sphinx-autodoc-typehints, and myst-parser, allowing any available versions. |
Sequence Diagram
sequenceDiagram
participant App as Application
participant Fetcher as Fetch Handler
participant ClientPool as HTTP Client Pool
participant TempClient as Temp Cookie Client
participant Parser as HTML Parser
participant Result as TeamInfo Result
App->>Fetcher: info(team_id=120)
Fetcher->>Fetcher: Create light mode cookies<br/>{dm: "0"}
Fetcher->>ClientPool: Check pooled client
ClientPool-->>Fetcher: Global client (no cookies)
Fetcher->>TempClient: Create temp client<br/>with light cookies
TempClient->>TempClient: fetch(url, cookies)
TempClient-->>Fetcher: HTML light mode
Fetcher->>Fetcher: Create dark mode cookies<br/>{dm: "1"}
Fetcher->>TempClient: Create temp client<br/>with dark cookies
TempClient->>TempClient: fetch(url, cookies)
TempClient-->>Fetcher: HTML dark mode
Fetcher->>Parser: Parse light HTML
Parser->>Parser: Extract logo_url_light
Parser-->>Fetcher: logo_url_light
Fetcher->>Parser: Parse dark HTML
Parser->>Parser: Extract logo_url_dark
Parser-->>Fetcher: logo_url_dark
Fetcher->>Result: Build TeamInfo
Result->>Result: Set logo_url = logo_url_light<br/>(backward compat)
Result-->>App: TeamInfo with all 3 URLs
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
🐰 Version bumped with care and delight,
Light mode, dark mode—logos shining bright!
Cookies guide the way, each request true,
Theme-specific logos for me and you! 🌙✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ❓ Inconclusive | The title 'Logo dark light urls' is vague and uses non-descriptive terms that don't clearly convey the specific changes made in the PR. | Consider using a more descriptive title like 'Add dark and light mode logo URLs to team info' to better communicate the main feature being added. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 93.33% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
logo_dark_light_urls
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 help to get the list of available commands and usage tips.
Deploying vlrdevapi with
|
| Latest commit: |
a03ddff
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a5021e85.vlrdevapi.pages.dev |
| Branch Preview URL: | https://logo-dark-light-urls.vlrdevapi.pages.dev |
![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)
pyproject.toml (1)
41-47:⚠️ Potential issue | 🟠 Major
pytestshould not be a runtime dependency.
pytest>=8.4.2is listed in the maindependencies, which means it will be installed for all users of the library. Test frameworks should only be in[project.optional-dependencies]undertestordev. This unnecessarily bloats the install for end users.Proposed fix
dependencies = [ "beautifulsoup4>=4.12", "httpx[brotli,http2]>=0.27", "lxml>=5.0", - "pytest>=8.4.2", "python-dateutil>=2.8.0", ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 41 - 47, The pyproject.toml currently lists "pytest>=8.4.2" inside the main dependencies list, which makes pytest a runtime dependency; remove "pytest>=8.4.2" from the dependencies array and add it under [project.optional-dependencies] (e.g., as a "test" or "dev" group) so tests are installed only for development; ensure the existing dependencies array (containing "beautifulsoup4", "httpx[...]","lxml","python-dateutil") remains unchanged and add a new optional-dependencies entry like test = ["pytest>=8.4.2"] if that section does not already exist.
🧹 Nitpick comments (5)
docs/requirements.txt (1)
1-5: Consider pinning or constraining documentation dependency versions.Removing version constraints from Sphinx-related packages can lead to reproducibility issues. Future installs may pull incompatible versions, causing doc builds to fail unexpectedly. Consider keeping minimum version constraints (e.g.,
sphinx>=7.0) or pinning exact versions for more deterministic builds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/requirements.txt` around lines 1 - 5, The docs requirements currently list unpinned packages (sphinx, sphinx-copybutton, sphinx-rtd-theme, sphinx-autodoc-typehints, myst-parser); change this to use version constraints to ensure reproducible builds—e.g., set a minimum Sphinx version (sphinx>=7.0) or pin exact versions for all listed packages (sphinx-copybutton, sphinx-rtd-theme, sphinx-autodoc-typehints, myst-parser) and update docs/requirements.txt accordingly so future installs pull compatible releases.tests/lib/conftest.py (1)
55-65: Other team URLs will use light mode HTML regardless of cookies.The current implementation only handles dark/light mode switching for team 120. Other team URLs (lines 56-63) will always return their mapped HTML file regardless of the
cookiesparameter. If future tests need dark/light mode testing for other teams, the mock will need to be extended.This is fine for now since only team 120 appears to have both variants, but worth noting for future test additions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lib/conftest.py` around lines 55 - 65, Update the mock URL-to-HTML mapping in the tests' conftest so team pages other than team 120 respect the cookies dark/light mode: modify the dict that currently contains entries like "https://www.vlr.gg/team/1034", "https://www.vlr.gg/team/8326", etc., and the mock response logic that checks cookies (where "https://www.vlr.gg/team/120" maps to "team_120_light.html") to return a dark-mode HTML variant when the cookies indicate dark mode; add corresponding dark-mode HTML fixtures (e.g., "team_1034_dark.html") and branch on the cookies parameter in the same function that picks "team_120_light.html" so all team URL keys can toggle between light and dark files.betterdocs/content/docs/api/teams/info.mdx (1)
96-118: Consider adding an example for theme-specific logos.The new
logo_url_lightandlogo_url_darkfields are documented in the type table but not demonstrated in the examples section. Adding a brief example would help users understand how to use these fields for theme-aware applications.📝 Suggested example addition after line 118
### Get Theme-Specific Logos ```python title="Get theme-specific logos" import vlrdevapi as vlr team = vlr.teams.info(team_id=1034) if team: # Use appropriate logo based on your app's theme print(f"Light mode logo: {team.logo_url_light}") print(f"Dark mode logo: {team.logo_url_dark}") # logo_url defaults to light mode for backward compatibility assert team.logo_url == team.logo_url_light</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@betterdocs/content/docs/api/teams/info.mdxaround lines 96 - 118, Add a
short example demonstrating the new theme-specific logo fields by extending the
Examples section (after the existing "Get Team Information" example) to show how
to read team.logo_url_light and team.logo_url_dark and to note that
team.logo_url defaults to light mode; include a tiny Python snippet that fetches
vlr.teams.info(team_id=...) and prints "Light mode logo: {team.logo_url_light}"
and "Dark mode logo: {team.logo_url_dark}" and an assertion or comment that
team.logo_url == team.logo_url_light for backward compatibility.</details> </blockquote></details> <details> <summary>src/vlrdevapi/teams/info.py (2)</summary><blockquote> `66-85`: **Optional: Extract logo extraction into a helper function.** The logo extraction logic is duplicated for light and dark modes. A small helper could reduce repetition. <details> <summary>♻️ Suggested helper extraction</summary> ```python def _extract_logo_url(header: BeautifulSoup | None) -> str | None: """Extract logo URL from team header element.""" if not header: return None logo_img = header.select_one(".team-header-logo img") if logo_img: src_val = logo_img.get("src") src = src_val if isinstance(src_val, str) else None if src: return absolute_url(src) return None # Usage: logo_url_light = _extract_logo_url(header_light) logo_url_dark = _extract_logo_url(header_dark)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vlrdevapi/teams/info.py` around lines 66 - 85, Refactor the duplicated logo extraction into a small helper (e.g., _extract_logo_url) that accepts a header node (header_light/header_dark) and returns a str|None; replace the duplicated blocks that set logo_url_light and logo_url_dark with calls to this helper and keep the final assignment logo_url = logo_url_light for backward compatibility. Ensure the helper checks for a None header, selects ".team-header-logo img", validates the src is a string, and returns absolute_url(src) or None.
36-51: Dual fetch adds latency; consider parallel fetching for performance.Each call to
vlr.teams.info()now makes two sequential HTTP requests (light mode then dark mode). This effectively doubles the latency for users who need team info. While the fallback logic is sound, consider fetching both modes concurrently to mitigate the latency impact.⚡ Proposed concurrent fetch using batch_fetch_html or ThreadPoolExecutor
def info(team_id: int, timeout: float | None = None) -> TeamInfo | None: ... url = f"{_config.vlr_base}/team/{team_id}" effective_timeout = timeout if timeout is not None else _config.default_timeout - - # Fetch page with light mode (dm=0) - try: - html_light = fetch_html(url, effective_timeout, cookies={"dm": "0"}) - except NetworkError: - return None - - # Fetch page with dark mode (dm=1) - try: - html_dark = fetch_html(url, effective_timeout, cookies={"dm": "1"}) - except NetworkError: - html_dark = html_light # Fallback to light mode if dark mode fails + + # Fetch both modes concurrently using threads + from concurrent.futures import ThreadPoolExecutor, as_completed + + html_light: str | None = None + html_dark: str | None = None + + def fetch_with_mode(mode: str) -> tuple[str, str | None]: + try: + return mode, fetch_html(url, effective_timeout, cookies={"dm": mode}) + except NetworkError: + return mode, None + + with ThreadPoolExecutor(max_workers=2) as executor: + futures = [executor.submit(fetch_with_mode, m) for m in ("0", "1")] + for future in as_completed(futures): + mode, html = future.result() + if mode == "0": + html_light = html + else: + html_dark = html + + if html_light is None: + return None + + if html_dark is None: + html_dark = html_light # Fallback🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vlrdevapi/teams/info.py` around lines 36 - 51, The current vlr.teams.info() implementation performs two sequential fetches using fetch_html (first cookies={"dm":"0"} then cookies={"dm":"1"}), doubling latency; refactor to run both fetches concurrently (e.g., via batch_fetch_html if available or ThreadPoolExecutor/futures) so html_light and html_dark are fetched in parallel, preserve the fallback behavior by using html_light if the dark fetch raises NetworkError, and keep exception handling around each future to map NetworkError to the intended fallback; update references to fetch_html, NetworkError, html_light, html_dark and the creation of soup_light/soup_dark accordingly.
🤖 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/vlrdevapi/teams/info.py`:
- Around line 75-82: The code may call header_dark.select_one when header_dark
can be None; update the logic around header_dark, logo_img_dark, and
logo_url_dark so you first check header_dark is not None (or truthy) before
calling select_one(".team-header-logo img"); if header_dark is None, ensure
logo_img_dark stays None and logo_url_dark remains None, otherwise proceed to
get src, validate it's a str, and call absolute_url(src); reference the
variables header_dark, logo_img_dark, logo_url_dark and the function
absolute_url when making the guard.
---
Outside diff comments:
In `@pyproject.toml`:
- Around line 41-47: The pyproject.toml currently lists "pytest>=8.4.2" inside
the main dependencies list, which makes pytest a runtime dependency; remove
"pytest>=8.4.2" from the dependencies array and add it under
[project.optional-dependencies] (e.g., as a "test" or "dev" group) so tests are
installed only for development; ensure the existing dependencies array
(containing "beautifulsoup4", "httpx[...]","lxml","python-dateutil") remains
unchanged and add a new optional-dependencies entry like test =
["pytest>=8.4.2"] if that section does not already exist.
---
Nitpick comments:
In `@betterdocs/content/docs/api/teams/info.mdx`:
- Around line 96-118: Add a short example demonstrating the new theme-specific
logo fields by extending the Examples section (after the existing "Get Team
Information" example) to show how to read team.logo_url_light and
team.logo_url_dark and to note that team.logo_url defaults to light mode;
include a tiny Python snippet that fetches vlr.teams.info(team_id=...) and
prints "Light mode logo: {team.logo_url_light}" and "Dark mode logo:
{team.logo_url_dark}" and an assertion or comment that team.logo_url ==
team.logo_url_light for backward compatibility.
In `@docs/requirements.txt`:
- Around line 1-5: The docs requirements currently list unpinned packages
(sphinx, sphinx-copybutton, sphinx-rtd-theme, sphinx-autodoc-typehints,
myst-parser); change this to use version constraints to ensure reproducible
builds—e.g., set a minimum Sphinx version (sphinx>=7.0) or pin exact versions
for all listed packages (sphinx-copybutton, sphinx-rtd-theme,
sphinx-autodoc-typehints, myst-parser) and update docs/requirements.txt
accordingly so future installs pull compatible releases.
In `@src/vlrdevapi/teams/info.py`:
- Around line 66-85: Refactor the duplicated logo extraction into a small helper
(e.g., _extract_logo_url) that accepts a header node (header_light/header_dark)
and returns a str|None; replace the duplicated blocks that set logo_url_light
and logo_url_dark with calls to this helper and keep the final assignment
logo_url = logo_url_light for backward compatibility. Ensure the helper checks
for a None header, selects ".team-header-logo img", validates the src is a
string, and returns absolute_url(src) or None.
- Around line 36-51: The current vlr.teams.info() implementation performs two
sequential fetches using fetch_html (first cookies={"dm":"0"} then
cookies={"dm":"1"}), doubling latency; refactor to run both fetches concurrently
(e.g., via batch_fetch_html if available or ThreadPoolExecutor/futures) so
html_light and html_dark are fetched in parallel, preserve the fallback behavior
by using html_light if the dark fetch raises NetworkError, and keep exception
handling around each future to map NetworkError to the intended fallback; update
references to fetch_html, NetworkError, html_light, html_dark and the creation
of soup_light/soup_dark accordingly.
In `@tests/lib/conftest.py`:
- Around line 55-65: Update the mock URL-to-HTML mapping in the tests' conftest
so team pages other than team 120 respect the cookies dark/light mode: modify
the dict that currently contains entries like "https://www.vlr.gg/team/1034",
"https://www.vlr.gg/team/8326", etc., and the mock response logic that checks
cookies (where "https://www.vlr.gg/team/120" maps to "team_120_light.html") to
return a dark-mode HTML variant when the cookies indicate dark mode; add
corresponding dark-mode HTML fixtures (e.g., "team_1034_dark.html") and branch
on the cookies parameter in the same function that picks "team_120_light.html"
so all team URL keys can toggle between light and dark files.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85244926-1c59-4104-894f-b1059904b7bd
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
betterdocs/app/(home)/page.tsxbetterdocs/content/docs/api/teams/info.mdxdocs/requirements.txtdocs/source/api/teams.rstdocs/source/conf.pypyproject.tomlsrc/vlrdevapi/__init__.pysrc/vlrdevapi/fetcher.pysrc/vlrdevapi/teams/info.pysrc/vlrdevapi/teams/models.pytests/html_sources/team_120_dark.htmltests/html_sources/team_120_light.htmltests/lib/conftest.pytests/lib/test_teams.py
Comment on lines
+75
to
+82
| # Extract logo URL for dark mode | ||
| logo_url_dark: str | None = None | ||
| logo_img_dark = header_dark.select_one(".team-header-logo img") | ||
| if logo_img_dark: | ||
| src_val = logo_img_dark.get("src") | ||
| src = src_val if isinstance(src_val, str) else None | ||
| if src: | ||
| logo_url_dark = absolute_url(src) |
There was a problem hiding this comment.
Potential None dereference if dark mode HTML lacks team header.
If the dark mode fetch succeeds but the HTML doesn't contain .team-header, header_dark will be None, and header_dark.select_one(...) on line 77 would raise AttributeError. The current code doesn't guard against this case since header_dark is used without a None check.
🛡️ Proposed fix to guard against None
# Extract logo URL for dark mode
logo_url_dark: str | None = None
- logo_img_dark = header_dark.select_one(".team-header-logo img")
- if logo_img_dark:
- src_val = logo_img_dark.get("src")
- src = src_val if isinstance(src_val, str) else None
- if src:
- logo_url_dark = absolute_url(src)
+ if header_dark:
+ logo_img_dark = header_dark.select_one(".team-header-logo img")
+ if logo_img_dark:
+ src_val = logo_img_dark.get("src")
+ src = src_val if isinstance(src_val, str) else None
+ if src:
+ logo_url_dark = absolute_url(src)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Extract logo URL for dark mode | |
| logo_url_dark: str | None = None | |
| logo_img_dark = header_dark.select_one(".team-header-logo img") | |
| if logo_img_dark: | |
| src_val = logo_img_dark.get("src") | |
| src = src_val if isinstance(src_val, str) else None | |
| if src: | |
| logo_url_dark = absolute_url(src) | |
| # Extract logo URL for dark mode | |
| logo_url_dark: str | None = None | |
| if header_dark: | |
| logo_img_dark = header_dark.select_one(".team-header-logo img") | |
| if logo_img_dark: | |
| src_val = logo_img_dark.get("src") | |
| src = src_val if isinstance(src_val, str) else None | |
| if src: | |
| logo_url_dark = absolute_url(src) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/vlrdevapi/teams/info.py` around lines 75 - 82, The code may call
header_dark.select_one when header_dark can be None; update the logic around
header_dark, logo_img_dark, and logo_url_dark so you first check header_dark is
not None (or truthy) before calling select_one(".team-header-logo img"); if
header_dark is None, ensure logo_img_dark stays None and logo_url_dark remains
None, otherwise proceed to get src, validate it's a str, and call
absolute_url(src); reference the variables header_dark, logo_img_dark,
logo_url_dark and the function absolute_url when making the guard.
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 CodeRabbit
New Features
logo_url_lightandlogo_url_dark) with backward compatibility via the existinglogo_urlfield.Documentation
Tests
Chores