fix: Allow asset download from private repos #24
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables downloading release assets from private GitHub repositories by using asset IDs and authentication tokens, while maintaining backward compatibility for public repos via a --public flag.
Key Changes:
- Added authentication support using GitHub tokens for private repository access
- Introduced
--publicflag to preserve original public-only download behavior - Modified asset download logic to use asset ID-based URLs for private repos
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/github.rs | Added id field to Asset struct, --public flag, and helper methods for repo metadata and token access |
| src/http/server.rs | Added authentication headers to HTTP client when accessing private repositories |
| src/http/service.rs | Implemented request builder with private/public repo logic and response inspection utilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let (_status_code, _content_length) = | ||
| Self::inspect_response(&resp); |
There was a problem hiding this comment.
The inspect_response call extracts status code and content length but doesn't use them (indicated by underscore prefix). If these values aren't needed for error handling or logging, consider removing this call.
There was a problem hiding this comment.
@markg-github is there a reason for not removing these? I agree w/ the copilot comments that I can't see why we'd want to inspect the response if nothing is done with the contents. Was this done for debug and then forgot to remove? The original implementation was pretty nice and concise.
src/http/service.rs
Outdated
| _ => client.get(url), // fallback; you only use HEAD/GET here | ||
| }; | ||
|
|
||
| if gh.is_private() { |
There was a problem hiding this comment.
I think this can be removed after you fix the other accept header
There was a problem hiding this comment.
Good catch. Thanks.
markg-github
force-pushed
the
private-repo-fix-pr
branch
from
fc4daae to
3994be4
Compare
Use --public to download in a way (the old way) that only works for public repos. Resolves: AMDEPYC#18
markg-github
force-pushed
the
private-repo-fix-pr
branch
from
3994be4 to
5836171
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let (_status_code, _content_length) = | ||
| Self::inspect_response(&resp); |
There was a problem hiding this comment.
The inspect_response function is called but its return values are unused (indicated by the underscore prefix). Consider removing this function call if the values aren't needed, or remove the underscores if they will be used in future iterations.
| let (_status_code, _content_length) = | |
| Self::inspect_response(&resp); | |
| // Removed unused inspect_response call |
| let (_status_code, _content_length) = | ||
| Self::inspect_response(&resp); |
There was a problem hiding this comment.
The inspect_response function is called but its return values are unused (indicated by the underscore prefix). Consider removing this function call if the values aren't needed, or remove the underscores if they will be used in future iterations.
| let (_status_code, _content_length) = | |
| Self::inspect_response(&resp); | |
| // Removed unused inspect_response call |
| status, | ||
| github, | ||
| client: Client::builder().redirect(policy).build()?, | ||
| client: client_builder.build()?, |
There was a problem hiding this comment.
Is there a reason that private would not need the redirect policy? Otherwise I think you keep the redirect() call here and remove the one at the top.
2 participants
Allow asset download from private repos: use URL with asset ID, supply token, etc.
Use --public to download in a way (the old way) that only works for public repos.
Resolves: #18