Add parallel discovery of Python versions for uv python list#18684
Draft
Add parallel discovery of Python versions for uv python list#18684
uv python list#18684Conversation
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 23, 2026
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
| true | ||
| } | ||
|
|
||
| /// In test mode, change the source to `Managed` if a version was marked as such via |
Member
Author
There was a problem hiding this comment.
Suggested change
| /// In test mode, change the source to `Managed` if a version was marked as such via | |
| /// In test mode, change the source to [`PythonSource::Managed`] if a version was marked as such via |
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
Comment on lines
+764
to
+770
| fn update_installation_source_for_tests(installation: &mut PythonInstallation) { | ||
| if std::env::var(uv_static::EnvVars::UV_INTERNAL__TEST_PYTHON_MANAGED).is_ok() | ||
| && installation.interpreter.is_managed() | ||
| { | ||
| installation.source = PythonSource::Managed; | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
Should we use #[must_use] add a with_source method that returns Self so we don't need a mut here?
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
| } | ||
|
|
||
| /// Check whether an installation satisfies the standard post-query discovery filters. | ||
| fn satisfies_discovery_filters( |
Member
Author
There was a problem hiding this comment.
Suggested change
| fn satisfies_discovery_filters( | |
| fn installation_satisfies_discovery_filters( |
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
Comment on lines
+740
to
+742
| if !interpreter_satisfies_environment_preference( | ||
| installation.source, | ||
| &installation.interpreter, |
Member
Author
There was a problem hiding this comment.
Can interpreter_satisfies_environment_preference be installation_satisfies_environment_preference and not take the decomposed values?
Member
Author
There was a problem hiding this comment.
It seems like it could also be EnvironmentPreference.allows_installation(...)?
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
Comment on lines
+747
to
+755
| let request = version.clone().into_request_for_source(installation.source); | ||
| if !request.matches_interpreter(&installation.interpreter) { | ||
| debug!( | ||
| "Skipping interpreter at `{}` from {}: does not satisfy request `{request}`", | ||
| installation.interpreter.sys_executable().user_display(), | ||
| installation.source, | ||
| ); | ||
| return false; | ||
| } |
Member
Author
There was a problem hiding this comment.
Should we abstract this with VersionRequest.matches_installation?
zanieb
added
the
build:skip-release
zanieb
commented
Mar 24, 2026
| } | ||
|
|
||
| /// Collect queried Python installations from executables, dropping non-critical discovery | ||
| /// errors. |
Member
Author
There was a problem hiding this comment.
We should have a comment linking to [iter_..]
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
|
|
||
| /// Find Python installations that satisfy the standard discovery filters, querying | ||
| /// interpreters concurrently. | ||
| fn find_matching_python_installations( |
Member
Author
There was a problem hiding this comment.
Suggested change
| fn find_matching_python_installations( | |
| fn find_all_matching_python_installations( |
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
|
|
||
| /// Create a [`PythonInstallation`] from a Python interpreter path. | ||
| fn python_installation_from_executable( | ||
| fn python_installation_from_provided_executable( |
Member
Author
There was a problem hiding this comment.
This method seems useless, we should just inline usage
zanieb
commented
Mar 24, 2026
crates/uv-python/src/discovery.rs
Outdated
Comment on lines
+1330
to
+1336
| if !preference.allows_source(PythonSource::ProvidedPath) { | ||
| return Err(Error::SourceNotAllowed( | ||
| request.clone(), | ||
| PythonSource::ProvidedPath, | ||
| preference, | ||
| )); | ||
| } |
Member
Author
There was a problem hiding this comment.
Could this be moved to preference.check_allows_request_source -> Result<(), Err)? so we can reuse it outside the match instead now that it's repeated?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.