Support proxy index urls #11782

jtfmumm · 2025-02-25T20:38:15Z

[This draft PR still has a couple of minor FIXMEs and a number of FIXMEs for documenting code. I also plan to add more tests]

These changes enable users to supply a proxy index url for an index/registry via an --index-proxy-url argument or UV_INDEX_PROXY_URL env var (both take values of the form <index-name>=<proxy-url>). The "canonical" URL for that index is defined via the normal url field in [[tool.uv.index]].

This canonical URL is written to the lockfile. But when a proxy url is passed in for that index, it will actually be used in place of the canonical URL (though never written to the lockfile). This way, if different users or machines have private proxy urls, these will never be checked into source control. Proxy URLs are supported for uv add, uv sync, uv lock, and uv run.

There is currently a test for adding a package with a proxy index and then removing .venv and syncing.

Closes #6349

charliermarsh · 2025-02-26T02:50:30Z

crates/uv-resolver/src/lock/mod.rs

-    pub fn to_toml(&self) -> Result<String, toml_edit::ser::Error> {
+    pub fn to_toml(
+        &self,
+        proxies: Option<&Vec<ProxyWithCanonicalUrl>>,


I haven't had a chance to review this yet, but as a minor note, I'd expect this to take Option<&[ProxyWithCanonicalUrl]> (and similarly for all public APIs). A slice is strictly more flexible than a reference to a Vec: if you have a vector, you can always pass it as a slice, but the opposite is not true.

charliermarsh · 2025-02-26T02:52:41Z

crates/uv/src/commands/project/lock_target.rs

@@ -268,6 +271,7 @@ impl<'lock> LockTarget<'lock> {
                                lock.version(),
                            ));
                        }
+                        let lock = lock.with_proxy_urls(proxies)?;


It seems like this should be composable -- in other words, should we do this outside of read, i.e., expect callers to call with_proxy_urls rather than passing it in as an argument and calling this method as the last step? It seems more flexible to me. Same for commit -- could we expect callers to apply the proxies before calling commit?

charliermarsh · 2025-02-26T02:52:54Z

crates/uv/src/commands/project/mod.rs

@@ -197,6 +197,9 @@ pub(crate) enum ProjectError {
    #[error("Attempted to drop a temporary virtual environment while still in-use")]
    DroppedEnvironment,

+    #[error("Failed to substitute proxy urls: {0}")]


We would stylize this as "URLs".

charliermarsh · 2025-02-26T02:55:40Z

crates/uv-distribution-types/src/index.rs

+    pub url: IndexUrl,
+    pub url_base: String,
+    pub canonical_url: IndexUrl,
+    pub raw_canonical_url: String,


Do these need to be stored on the struct? Can't they always be recovered via url.as_str()? (Also, does this mean they're part of the schema, serialized, deserialized, etc.? If so, is that intentional?)

It might help to add rustdoc-style comments to each field here to illustrate the expected values with an example.

charliermarsh · 2025-02-26T02:56:02Z

crates/uv-distribution-types/src/index.rs

+    #[error("Proxy index requires a name, but the name was empty")]
+    MissingName,
+    // #[error(transparent)]
+    // UrlParse(#[from] url::ParseError),


(Nit: dead code)

charliermarsh · 2025-02-26T02:58:35Z

crates/uv-resolver/src/lock/mod.rs

+                        .is_ok_and(|url| url.is_some_and(|url| url == p.url))
+                })
+            }) {
+                Some(proxy) => &dist.with_canonical_urls(proxy).expect("FIXME"),


If we had a method like with_canonical_urls that took mut self on the Lock, could we avoid having to re-clone the entire distribution here?

I've changed this and related methods to mutate directly without cloning

charliermarsh · 2025-02-26T03:00:52Z

Nice job figuring out all the plumbing here. As a higher-level comment, we should definitely get feedback from the users that participated in #6349 to understand whether the proposed API solves their problem (or not). I'd probably suggesting commenting there with a clear outline on the proposed design and a link to this PR sooner rather than later, so that we can make sure we're on the right track. In that comment, you could also ask if users are willing / able to test this branch out on their own projects.

zanieb · 2025-02-26T03:01:34Z

crates/uv-cli/src/lib.rs

@@ -4738,6 +4753,10 @@ pub struct IndexArgs {
    #[arg(long, env = EnvVars::UV_EXTRA_INDEX_URL, value_delimiter = ' ', value_parser = parse_extra_index_url, help_heading = "Index options")]
    pub extra_index_url: Option<Vec<Maybe<PipExtraIndex>>>,

+    /// FIXME Document
+    #[arg(long, env = EnvVars::UV_INDEX_PROXY_URL, value_delimiter = ' ', value_parser = parse_index_proxy_url, help_heading = "Index options")]


I worry about UV_INDEX_PROXY_URL not matching the UV_INDEX_<name>_USERNAME and UV_INDEX_<name>_PASSWORD options. Presumably, the reason the environment variables include the index name is it makes it easier to set variables for multiple indexes? We also may have just copied Poetry here. But, otherwise you have to pass them as separated values (with spaces here, commas in some other interfaces) — which is fine in the general case, but probably problematic for credentials where separators could be valid characters.

I'm not entirely sure this should change, but consistency is important. You could argue we should support UV_INDEX_USERNAME=<name>=<username> instead, but I don't know how we'd deal with the aforementioned case where the separator is a valid character in the value.

Another argument is that we don't have CLI options for usernames and passwords — they're part of the --index <...> value. It could be sufficiently distinct for that reason. We could weigh the usage of --index-proxy-url directly matching UV_INDEX_PROXY_URL over consistency with the other environment variables.

Another option is to support both, but I don't think there's strong justification for that.

zanieb · 2025-02-26T03:11:56Z

crates/uv/tests/it/edit.rs

+    uv_snapshot!(context.filters(), context.add()
+        .arg("anyio>=4.3.0")
+        .arg("--index-proxy-url")
+        .arg("alpha=https://public:[email protected]/basic-auth/simple"), @r"


Might be preferable not to use our proxy here, it's a bit flaky and I'd only use it if you're trying to test for proper credential management. You should just be able to use the normal PyPI?

Speaking of testing credentials, it may be worth having a separate test that uses the UV_INDEX... variables to set credentials and ensure they're propagated to the proxy URL correctly.

zanieb · 2025-02-26T03:13:38Z

crates/uv/tests/it/edit.rs

+        );
+    });
+
+    fs_err::remove_dir_all(context.temp_dir.join(".venv"))?;


Why remove the virtual environment? (Generally --reinstall will suffice). Are you just testing sync here? Couldn't we be pulling the package fro the cache?

I'd probably just write a dedicated sync test.

zanieb · 2025-02-26T03:14:20Z

crates/uv/tests/it/edit.rs

+    "
+    );
+
+    let pyproject_toml = context.read("pyproject.toml");


We should also snapshot the lockfile, right? Maybe that'd be a test case in crates/uv/tests/it/lock.rs?

WIP: Proxy index

5f01c0b

jtfmumm force-pushed the jtfm/url-proxy branch 2 times, most recently from 7d8690b to d219a7b Compare February 25, 2025 21:02

zanieb self-requested a review February 25, 2025 21:03

jtfmumm force-pushed the jtfm/url-proxy branch from d219a7b to 0aab145 Compare February 25, 2025 21:10

Lockfile proxying

0e06041

jtfmumm force-pushed the jtfm/url-proxy branch from 0aab145 to 0e06041 Compare February 25, 2025 21:25

charliermarsh reviewed Feb 26, 2025

View reviewed changes

zanieb reviewed Feb 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support proxy index urls #11782

Support proxy index urls #11782

jtfmumm commented Feb 25, 2025

charliermarsh Feb 26, 2025

charliermarsh Feb 26, 2025

jtfmumm Feb 26, 2025

charliermarsh Feb 26, 2025

charliermarsh Feb 26, 2025

charliermarsh Feb 26, 2025

charliermarsh Feb 26, 2025

charliermarsh Feb 26, 2025

jtfmumm Feb 26, 2025

charliermarsh commented Feb 26, 2025

zanieb Feb 26, 2025 •

edited

Loading

zanieb Feb 26, 2025

zanieb Feb 26, 2025

zanieb Feb 26, 2025

zanieb Feb 26, 2025

Support proxy index urls #11782

Are you sure you want to change the base?

Support proxy index urls #11782

Conversation

jtfmumm commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh commented Feb 26, 2025

zanieb Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanieb Feb 26, 2025 •

edited

Loading