feat(pytorch-wheels): proxy index pages#84
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the PyTorch wheels “pytorch-wheels” repository handling to proxy index pages (rather than redirecting them), and adjusts the Rust/reqwest dependency/build setup to address musl-related TLS build issues. It also introduces a new PyPI-index crawler/index cache to support dynamic proxying behavior.
Changes:
- Add a new
pypi_indexmodule that crawls PyTorch’s wheel index pages, persists discovered index entries, and refreshes them periodically. - Update routing/proxy logic to proxy index pages (and HEAD responses) before applying filter-based redirects for the
pytorch-wheelsroute. - Simplify reqwest client creation across the codebase and update Nix build configuration for musl/static builds; add HTML fixtures for index-crawling tests.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pytorch_wheels/whl.html | Adds fixture for root wheels index parsing tests. |
| tests/pytorch_wheels/whl_cu130.html | Adds fixture for CUDA 13.0 subtree index parsing tests. |
| tests/pytorch_wheels/whl_cu130_torch.html | Adds fixture for torch links under cu130/ for crawler tests. |
| src/utils.rs | Tests now build reqwest clients directly via Client::new(). |
| src/repos.rs | Reorders proxy vs filter logic; expands wheels_proxy and adds a new HEAD proxy test. |
| src/queue.rs | Tests now build reqwest clients directly via Client::new(). |
| src/pypi_index.rs | New crawler + persisted/refreshing index implementation and tests. |
| src/main.rs | Uses reqwest::Client/ClientBuilder directly; wires in pypi_index module. |
| src/common.rs | Removes custom rustls provider installation + reqwest client factory helpers. |
| src/artifacts.rs | Tests now build reqwest clients directly via Client::new(). |
| README.md | Minor wording tweak in request flow description. |
| nix/crane.nix | Uses a fileset-based source including tests/; adds mergeCraneArgs helper. |
| flake.nix | Uses mergeCraneArgs for musl build inputs; enables taplo; adds devShell tooling. |
| deny.toml | Allows MPL-2.0 licenses. |
| Cargo.toml | Switches reqwest feature config and adds scraper dependency. |
| Cargo.lock | Updates lockfile for new dependency graph (incl. scraper + TLS-related deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Index page persistance should be in-memory so as to eliminate disk I/O overhead. Extra memory usage can be limited to MiB level, which is fairly enough at least for pytorch-wheels. |
36372d2 to
7640392
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/common.rs:339
Redirect::Permanent/Temporarydoc comments state 308/307, but the actual conversion uses 301/302 (and now notes this for compatibility). Consider updating the enum/variant docs to match the real status codes to avoid confusion for callers.
/// An empty redirect response to a given URL.
pub enum Redirect {
/// Construct a “permanent” (308) redirect response.
Permanent(String),
/// Construct a “temporary” (307) redirect response.
Temporary(String),
}
impl From<Redirect> for HttpResponse {
fn from(this: Redirect) -> Self {
// NOTE: use 301 and 302 for compatibility with old clients, even though they are not strictly correct.
let (code, location) = match this {
Redirect::Permanent(url) => (StatusCode::MOVED_PERMANENTLY, url),
Redirect::Temporary(url) => (StatusCode::FOUND, url),
};
Self::build(code)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.