Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support private github repository #1690

Open
wants to merge 54 commits into
base: main
Choose a base branch
from
Open

Support private github repository #1690

wants to merge 54 commits into from
+1,839 −1,129

Conversation

NobodyXu
Copy link
Member

@NobodyXu NobodyXu commented May 5, 2024

Fixed #1681

@scullionw
Copy link

is there anything we can do to help move this forward? maybe testing a build?

@NobodyXu
Copy link
Member Author

is there anything we can do to help move this forward? maybe testing a build?

I just need to spend more time on this, I'm a bit busy recently, but rest asure, I didn't forget it.

@NobodyXu
Copy link
Member Author

Update: I'm a bit busy this week.

cc @scullionw we actually need more contributors for cargo-binstall.

If you are willing to submit a PR, then I can do a code review very quickly.

If you have time, you could pick up where I have left and I'm willing to answer any questions you have related to the codebase.

@NobodyXu
Merge branch 'main' into feat/private-github-repo
c36145f
@NobodyXu
Refactor: Make sure 'static Future is returned
cb418fd 
To make it easier to create generic function

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add logging to unit testing
fd5b78c 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix unit testing
361aca4 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor: Extract new fn GhApiClient::do_fetch
1350ac3 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Rm unused percent_encode_http_url_path
ffe47d3 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix cargo test run on CI
962a7d0 
`cargo test` run all tests in one process.

As such, `set_global_default` would fail on the second call.

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Optimize GhApiClient::do_fetch: Avoid unnecessary restful API call
fc02228 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Refactor: Rm param auth_token for restful API fn
f824ebb 
which is always set to `None`

Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Remove unused deps added to binstalk-git-repo-api
54f88e0 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix clippy lints
1ad2d0a 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add new API GhApiClient::remote_client
6b0f460 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add GhApiClient::has_gh_token
7e283cc 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add GhRepo::try_extract_from_url
81bdc3b 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Rename ReleaseArtifactUrl to GhReleaseArtifactUrl
3cee8a9 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add new fn Download::with_data_verifier
c19fcb6 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
feature: Support private repository
0838118 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix clippy lints
fba7e01 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add e2e-test/private-github-repo
be3aeca 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix clippy lints
53b3c02 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix launch_baseline_find_tasks: Retry on rate limit
6b3e384 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix test failure: Retry on rate limit
c9fb2f4 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Temporarily enable debug output for e2e-test-private-github-repo
d1aa727 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Fix get_repo_info: Retry on rate limit
bc71a1b 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Improve debug! logging
a295fe9 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add more debug logging
e02321e 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add more debugging
23a1bed 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Add more debug logging
a236f53 
Signed-off-by: Jiahao XU <[email protected]>
@NobodyXu
Copy link
Member Author

NobodyXu commented Jun 9, 2024

When debugging CI failure, I've found:

DEBUG has_release_artifact{release=GhRelease { repo: GhRepo { owner: "cargo-bins", repo: "private-repo-for-testing" }, tag: "0.12.0" } artifact_name="cargo-binstall-x86_64-unknown-linux-musl-v0.12.0.tar"}: response = GraphQLResponse { data: GraphQLData { repository: Some(GraphQLRepo { release: None }) }, errors: None }

seems that somehow release artifact GraphQL cannot found the release in the repository, despite it being present in the repo.

@NobodyXu
Copy link
Member Author

NobodyXu commented Jun 9, 2024

Ok turns out that I forgot to give the token enough permission.

I've recreated github token, using another machine account which only has read-only access to the repository, and give the new token full access to the private repostories of that machine account, since it has read-only access anyway.

NobodyXu
NobodyXu commented Jun 9, 2024
View reviewed changes
e2e-tests/private-github-repo.sh Outdated Show resolved Hide resolved
crates/binstalk-git-repo-api/src/gh_api_client/release_artifacts.rs Outdated Show resolved Hide resolved
crates/binstalk-git-repo-api/src/gh_api_client.rs Outdated Show resolved Hide resolved
crates/binstalk-git-repo-api/src/gh_api_client.rs Outdated Show resolved Hide resolved
@NobodyXu NobodyXu marked this pull request as ready for review June 9, 2024 14:48
@NobodyXu NobodyXu requested a review from passcod June 9, 2024 14:59
@NobodyXu
Copy link
Member Author

cc @scullionw This PR is now ready for review, you should be able to download from private repositories using this PR, could you have a try please?

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

@passcod passcod Awaiting requested review from passcod

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: cannot access private repositor with GITHUB_TOKEN provided
2 participants
@NobodyXu @scullionw