Skip to content

Conversation

@h0nIg
Copy link
Contributor

@h0nIg h0nIg commented Sep 8, 2025

Summary by CodeRabbit

  • New Features
    • Added support for GitHub Enterprise and other non-default GitHub hosts when fetching releases. The app now auto-detects the repository host and uses the appropriate API endpoint, preserving existing authentication and parsing behavior. Behavior for github.com remains unchanged. No changes to public interfaces or configuration are required.

@coderabbitai
Copy link

coderabbitai bot commented Sep 8, 2025

Walkthrough

Implements conditional construction of the GitHub releases API URL to support non-default GitHub hosts while keeping existing token handling, request execution, and JSON-to-Version parsing unchanged. Moves the pagination TODO comment to align with the new URL branch.

Changes

Cohort / File(s) Summary
GitHub API URL handling
nix_update/version/github.py
Adds host-aware API base selection: uses https://api.github.com/... for default hosts and https://{netloc}/api/v3/... for other hosts; retains request/auth/parsing flow; relocates pagination TODO.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant GH as GitHubReleasesFetcher
    participant URL as URL Builder
    participant HTTP as HTTP Client
    participant JSON as Parser

    Caller->>GH: fetch_releases(url, token)
    GH->>URL: build_releases_api_url(owner, repo, netloc)
    alt Default host (github.com/api.github.com)
        URL-->>GH: https://api.github.com/repos/{owner}/{repo}/releases?per_page=100
    else Non-default host (Enterprise)
        URL-->>GH: https://{netloc}/api/v3/repos/{owner}/{repo}/releases?per_page=100
    end
    GH->>HTTP: GET releases (Authorization if token)
    HTTP-->>GH: JSON response
    GH->>JSON: parse releases into Version objects
    JSON-->>Caller: [Version...]
    note over GH,HTTP: TODO: pagination handling remains pending
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I sniffed the nets with whiskered flair,
Found hosts beyond the common lair.
Two paths to fetch the tags we seek,
Default, Enterprise—both sleek.
With tokens tucked, I hop with ease,
New releases plucked like clover leaves. 🌿🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the primary change—adding support for GitHub releases on on-premise/self-hosted installations—and directly reflects the modifications in github.py, so it represents the main intent of the PR.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Collaborator

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested with GH

gh api --hostname github.some.domain repos/abc/def/releases?per_page=100

# TODO: pagination?
github_url = f"https://api.github.com/repos/{owner}/{repo}/releases?per_page=100"
if url.netloc != "github.com" and url.netloc != "api.github.com":
github_url = f"https://{url.netloc}/api/v3/repos/{owner}/{repo}/releases?per_page=100"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for still beeing in draft mode?

@Mic92 Mic92 marked this pull request as ready for review September 15, 2025 07:28
@Mic92 Mic92 enabled auto-merge September 15, 2025 07:28
@Mic92 Mic92 merged commit a50ac52 into Mic92:main Sep 15, 2025
3 of 4 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
nix_update/version/github.py (1)

91-91: Consider the pagination implementation for production use.

The pagination TODO comment has been moved here, which aligns well with the new URL construction logic. However, for production use with large repositories or on-premise installations that may have many releases, pagination should be implemented to ensure all releases are fetched.

Here's a potential implementation approach for pagination:

def fetch_github_versions_from_releases(
    url: ParseResult,
    owner: str,
    repo: str,
) -> list[Version]:
    base_url = f"https://api.github.com/repos/{owner}/{repo}/releases"
    if url.netloc not in {"github.com", "api.github.com"}:
        base_url = f"https://{url.netloc}/api/v3/repos/{owner}/{repo}/releases"
    
    token = os.environ.get("GITHUB_TOKEN")
    extra_headers = {} if token is None else {"Authorization": f"Bearer {token}"}
    
    all_versions = []
    page = 1
    per_page = 100
    
    while True:
        github_url = f"{base_url}?per_page={per_page}&page={page}"
        info(f"fetch {github_url}")
        resp = _dorequest(url, github_url, extra_headers)
        
        try:
            releases = json.loads(resp)
        except json.JSONDecodeError:
            info("unable to parse github response, ignoring")
            break
            
        if not releases:  # No more releases
            break
            
        all_versions.extend([Version(r["tag_name"], r["prerelease"]) for r in releases])
        
        if len(releases) < per_page:  # Last page
            break
            
        page += 1
    
    return all_versions
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c9122c and 68ce39d.

📒 Files selected for processing (1)
  • nix_update/version/github.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: tests
🔇 Additional comments (1)
nix_update/version/github.py (1)

86-89: LGTM! Proper conditional URL construction for on-premise GitHub installations.

The logic correctly handles different GitHub hosts by checking if the netloc is not the default GitHub domains and constructs the appropriate API URL for on-premise installations using the /api/v3/ path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants