Only require setuptools when an initial attempt fails#270
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| // | ||
| // https://github.com/pypa/setuptools/issues/5174 | ||
| let unpinned_resolves = { | ||
| let mut probe = Command::new(&self.uv_path); |
There was a problem hiding this comment.
Actually this solution disguises possibly a major latency increase for the happy path when the user app does not pin setuptools in requirements.txt it seems as there is an additional round trip to pypi to resolve
There was a problem hiding this comment.
This is typically really fast; however, I generally agree that we have to go to PyPI twice. Do you have another suggestion for how we can do this with only one round trip?
There was a problem hiding this comment.
I think I have a solution actually. One sec.
| return Err(e.into()); | ||
| } | ||
| Ok(mut retry_child) => { | ||
| let stdout = retry_child.stdout.take().expect("no stdout"); |
There was a problem hiding this comment.
could probably take this whole bit out into its own helper function now, but not a blocker
| /// Returns whether a failed `sync()` for this directory is eligible for a | ||
| /// retry via [`sync_with_legacy_setuptools_pin`]. Only applies to projects | ||
| /// driven by `requirements.txt`; pyproject-based projects manage their own | ||
| /// setuptools dependency. | ||
| pub fn should_use_legacy_setuptools_pin(&self, cwd: &Path) -> bool { | ||
| cwd.join("requirements.txt").exists() | ||
| } |
There was a problem hiding this comment.
I think if we're doing the retesting method we have no reason to differentiate
3 participants
We had some overly-aggressive logic that tried to resolve a known issue with setuptools in the Python ecosystem. This PR makes that slightly more efficient in the default case with a reasonable fallback when something goes wrong.