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.

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

feat(cargo): set user-agent for crate api request #1933

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

olblak
Copy link
Member

@olblak olblak commented Feb 15, 2024

Since recently, the crate.io API requires a token to make queries.
This pullrequest add a new user agent "Updatecli" when interacting with the crate api

I also took the opportunity to update target name to be conventional commit compliant as they are used to create commit title.

Move test from a sub package directly to the cargo autodiscovery package, so go test can show the real test coverage`

Test

To test this pull request, you can run the following commands:

cd pkg/plugins/resources/cargopackage/
go test
go build -o bin/updatecli .
./bin/updatecli diff --config e2e/updatecli.d/success.d/cargopackage.yaml 

Additional Information

Tradeoff

The tradeoff to switch to the git repository by default means we first need to clone the repository locally which is a few gibabytes but when we need to update a lot of crate packages, it turns out being faster because we clone once per updatecli execution and then we can reuse information locally a checkout is done at the beginning of each target. While with the api, we are affected by rate limit to one request per second. cfr

Performance were horrible so I went back using the API with a user-agent
But I kept the improvement in core to clone scm defined by autodiscovery plugins as it will be useful in the futur

Potential improvement

  • Use a token to test the crate API

@olblak olblak added chore resource-cargo Related to Rust crate labels Feb 15, 2024
@olblak olblak changed the title feat(cargo): disable default api test feat(cargo): default registry relies on git instead of api Feb 16, 2024
@olblak olblak changed the title feat(cargo): default registry relies on git instead of api feat(cargo): default cargo registry uses Git protocol Feb 16, 2024
@loispostula
Copy link
Contributor

@olblak looks good to me, we initially went for the api to not have to clone a multi gig repo to perform the search.

When first implementing I investigated using sparse checkout as the name path of the crate in the index is predictable. But back then our go git library was not supporting that. It looks like it' s now supported, should we use it? https://pkg.go.dev/github.com/go-git/go-git/v5#CheckoutOptions

By default, the cargo autodiscovery uses the crate.io-index git repository https://github.com/rust-lang/crates.io-index.git which is the recommended approach the cargo project.

We also introduce the following minor changes:

* Update target name to be conventional commit compliant as they are used to create commit title.
* Move test from a sub package directly to the cargo autodiscovery package, so go test can show the real test coverage

Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member Author

olblak commented Feb 16, 2024

@olblak looks good to me, we initially went for the api to not have to clone a multi gig repo to perform the search.

When first implementing I investigated using sparse checkout as the name path of the crate in the index is predictable. But back then our go git library was not supporting that. It looks like it' s now supported, should we use it? https://pkg.go.dev/github.com/go-git/go-git/v5#CheckoutOptions

That's great to know, that being said some improvement will also be needed in the core of Updatecli because we pretty much always run pull operation when doing a clone or checkout.
While it makes sense for some update scenario, in the context of cargo it just slow down everything

Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
@olblak olblak changed the title feat(cargo): default cargo registry uses Git protocol feat(cargo): set user-agent for crate api request Feb 16, 2024
@olblak olblak merged commit 2ff654d into updatecli:main Feb 16, 2024
6 checks passed
@olblak olblak deleted the cargo/disableApiTest branch April 3, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore resource-cargo Related to Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants