Simplify crates.io API caching #631
Merged
Conversation
This file contains 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
|
Note that this change also starts collecting the sha256 hashes for versions when fetching from the index. I haven't implemented actually validating these when downloading yet (though that should be done at some point, and likely won't be too difficult, beyond requiring adding & auditing new dependencies). |
afranchuk
approved these changes
Sep 4, 2024
src/storage.rs
Outdated
| } | ||
| let mut result = SortedMap::new(); | ||
| for version_entry in crate_entry.versions() { | ||
| if let Ok(version) = semver::Version::parse(version_entry.version()) { |
There was a problem hiding this comment.
Maybe log this error? It should be unlikely, though.
Previously, we would perform time-based caching for data from the crates.io API in order to avoid unnecessary load on it. Unfortunately, this would sometimes lead to situations such as mozilla#629 where the cache is out-of-date and confusing errors are produced. Since the last time we looked at this, the API guidance for the sparse HTTP index has been clarified, explicitly having "no rate limit". This allows us to simplify the caching code dramatically, caching only the data from the rate-limited crates.io API, and always checking the sparse HTTP index to see if new versions have been published. The new caching system no longer needs expiry times, as it will be updated consistently whenever a new version is published to the crates.io index. After this change, we will make more network requests than before when running cargo-vet to check the index.
mystor
force-pushed
the
simplify_crates_caching
branch
from
9887560 to
b8500de
Compare
mystor
added a commit
to mystor/cargo-vet
that referenced
this pull request
Sep 18, 2024
These #[instrument] directives were added in mozilla#631, and end up logging error messages for every crate which isn't on crates.io due to CrateInfoError::DoesNotExist. Remove these instrument lines to avoid the unnecessary error spam.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously, we would perform time-based caching for data from the crates.io API in order to avoid unnecessary load on it. Unfortunately, this would sometimes lead to situations such as #629 where the cache is out-of-date and confusing errors are produced.
Since the last time we looked at this, the API guidance for the sparse HTTP index has been clarified, explicitly having "no rate limit". This allows us to simplify the caching code dramatically, caching only the data from the rate-limited crates.io API, and always checking the sparse HTTP index to see if new versions have been published.
The new caching system no longer needs expiry times, as it will be updated consistently whenever a new version is published to the crates.io index.
After this change, we will make more network requests than before when running cargo-vet to check the index.