Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow fallback to Python download on non-critical discovery errors #10908

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Allow fallback to Python download on non-critical discovery errors #10908

merged 1 commit into from
Jan 23, 2025

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Jan 23, 2025 •
edited
Loading

Closes #10898

In #10716, I broke fallback to downloading Python versions by throwing a different error kind.

@zanieb zanieb added the bug Something isn't working label Jan 23, 2025
@zanieb zanieb changed the title zb/find download Allow fallback to Python download on non-critical discovery errors Jan 23, 2025
zanieb
zanieb commented Jan 23, 2025
View reviewed changes
crates/uv-python/src/installation.rs
Comment on lines +111 to +112
// If we raised a non-critical error, we should attempt a download
Error::Discovery(ref err) if !err.is_critical() => {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change. The rest is just refactoring so we can match on multiple error types cleanly.

@zanieb
Copy link
Member Author

zanieb commented Jan 23, 2025

I want to add test coverage, but reviews are welcome — this is important to get out today.

@zanieb zanieb marked this pull request as ready for review January 23, 2025 18:13
zanieb
zanieb commented Jan 23, 2025
View reviewed changes
crates/uv-python/src/installation.rs
match Self::find(request, environments, preference, cache) {
Ok(venv) => Ok(venv),
// If missing and allowed, perform a fetch
Err(Error::MissingPython(err))
Copy link
Member Author

@zanieb zanieb Jan 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#10716 added a throw of Error::Discovery instead of a PythonNotFound error (which is mapped to MissingPython transparently here) which means we no longer fallback.

Ideally we'd implement #10716 higher up, like here? but couldn't think of a clean way to do that.

@zanieb zanieb mentioned this pull request Jan 23, 2025
Merged
@zanieb zanieb changed the base branch from main to zb/install-auto-test January 23, 2025 21:08
@zanieb zanieb force-pushed the zb/install-auto-test branch 5 times, most recently from e7d5324 to e9043f2 Compare January 23, 2025 22:13
Base automatically changed from zb/install-auto-test to main January 23, 2025 22:21
@zanieb zanieb enabled auto-merge (squash) January 23, 2025 22:35
@zanieb zanieb merged commit cbf6d5a into main Jan 23, 2025
72 checks passed
@zanieb zanieb deleted the zb/find-download branch January 23, 2025 22:37
@ceejatec
Copy link

Thanks for this! We hit this bug just today, and I was literally in the process of writing up a bug report ten minutes ago and couldn't figure out how to reproduce it. Turns out we hit the bug with uv 0.5.23 an hour ago, but when I was working on a reproducer it got the fresh-from-the-oven uv 0.5.24.

@zanieb
Copy link
Member Author

zanieb commented Jan 24, 2025

That's great to hear! I figured someone else would hit it quickly 😬

@BrewTestBot BrewTestBot mentioned this pull request Jan 24, 2025
Merged
@jramcast
Copy link

Thank you for the quick fix! All is working nicely now.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jan 26, 2025
@tmeijn
build(deps): update astral-sh/uv to v0.5.24
d345d30 
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.5.22` -> `0.5.24` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.5.24`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0524)

[Compare Source](astral-sh/uv@0.5.23...0.5.24)

##### Enhancements

-   Improve determinism of resolution by always setting package priorities ([#&#8203;10853](astral-sh/uv#10853))
-   Upgrade to `cargo-dist` 0.28.0; improves several installer behaviors ([#&#8203;10884](astral-sh/uv#10884))

##### Performance

-   Remove dependencies clone in resolver ([#&#8203;10880](astral-sh/uv#10880))
-   Use Hashbrown's raw entry API to reduce hashes and clone in resolver priority determination ([#&#8203;10881](astral-sh/uv#10881))

##### Bug fixes

-   Allow fallback to Python download on non-critical discovery errors ([#&#8203;10908](astral-sh/uv#10908))

##### Preview features

-   Register managed Python version with the Windows Registry (PEP 514) ([#&#8203;10634](astral-sh/uv#10634))

##### Documentation

-   Improve documentation for some environment variables ([#&#8203;10887](astral-sh/uv#10887))
-   Add git subdirectory example ([#&#8203;10894](astral-sh/uv#10894))

### [`v0.5.23`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0523)

[Compare Source](astral-sh/uv@0.5.22...0.5.23)

##### Enhancements

-   Add `--refresh` to `uv venv` ([#&#8203;10834](astral-sh/uv#10834))
-   Add `--no-default-groups` command-line flag ([#&#8203;10618](astral-sh/uv#10618))

##### Bug fixes

-   Sort extras and groups when comparing lockfile requirements ([#&#8203;10856](astral-sh/uv#10856))
-   Include `commit_id` and `requested_revision` in `direct_url.json` ([#&#8203;10862](astral-sh/uv#10862))
-   Invalidate lockfile when static versions change ([#&#8203;10858](astral-sh/uv#10858))
-   Make GitHub fast path errors non-fatal ([#&#8203;10859](astral-sh/uv#10859))
-   Remove warnings for `--frozen` and `--locked` in `uv run --script` ([#&#8203;10840](astral-sh/uv#10840))
-   Resolve `find-links` paths relative to the configuration file ([#&#8203;10827](astral-sh/uv#10827))
-   Respect visitation order for proxy packages ([#&#8203;10833](astral-sh/uv#10833))
-   Treat version mismatch errors as non-fatal in fast paths ([#&#8203;10860](astral-sh/uv#10860))
-   Mark `--locked` and `--upgrade` are conflicting ([#&#8203;10836](astral-sh/uv#10836))
-   Relax error checking around unconditional enabling of conflicting extras ([#&#8203;10875](astral-sh/uv#10875))

##### Documentation

-   Reduce ambiguity in conflicting extras example ([#&#8203;10877](astral-sh/uv#10877))
-   Update pre-commit documentation  ([#&#8203;10756](astral-sh/uv#10756))

##### Error messages

-   Error when workspace contains conflicting Python requirements ([#&#8203;10841](astral-sh/uv#10841))
-   Improve uvx error message when uv is missing ([#&#8203;9745](astral-sh/uv#9745))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xMjIuMCIsInVwZGF0ZWRJblZlciI6IjM5LjEyNC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
This was referenced Jan 28, 2025
Closed
Merged
zanieb added a commit that referenced this pull request Jan 28, 2025
@zanieb
Fix best-interpreter lookups when there is an invalid interpreter in …
e26affd 
…the PATH (#11030)

Closes #10978

The root cause is the same as #10908 — I should have been more careful
with the original change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gankra Gankra Gankra approved these changes

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

uv --python and uv python pin fail if Python3.6 is found on the system
5 participants
@zanieb @ceejatec @jramcast @Gankra @charliermarsh