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.

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

Use UV_SYSTEM_PYTHON to allow the system Python interpreter #7317

Merged
merged 1 commit into from
May 24, 2024

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented May 23, 2024

Hi!

I noticed this repository linked in astral-sh/uv#3765 and see that you're having trouble with the breaking change we released. For some more context, you were using the VIRTUAL_ENV variable to point to a system installation which was a way to bypass our lack of system Python support. However, in the latest release we added stricter validation of virtual environments so if VIRTUAL_ENV points to a system Python interpreter, we will ignore it. Of course, in CI you don't care about virtual environments and it's totally fine to mutate the system Python — for that reason we added --system support a while ago.

I saw in #7309 you tried to thread through a --system flag to opt-in to using the system interpreter. That's the right idea, but it's easy to miss a case — which will cause uv to fail as it did there. Instead, you can use the UV_SYSTEM_PYTHON flag to opt-in without threading the argument anywhere. I think this should solve your problems, if it doesn't I'll definitely look into it. I've added an integration test for this pattern to be sure we have extra coverage of it astral-sh/uv#3805. As a minor note, you could probably use UV_PYTHON with the path to the interpreter as well as we'll allow you to explicitly select the system interpreter that way.

In this pull request, I also add an upper bound to uv so you shouldn't encounter any breaking changes unless you opt-in to a later version. Note, you could avoid installing pip in CI at all and use our curl installer with a pinned uv version e.g. curl -LsSf https://astral.sh/uv/0.2.2/install.sh | sh but then you can't use version ranges so 🤷‍♀️

Of course, feel free to disregard this if you're not interested.

Copy link

netlify bot commented May 23, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit 7431d0c
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/664fd4b13b24cd000899375f

@SchrodingersGat
Copy link
Member

@zanieb thanks for getting in touch, and submitting a potential fix for us :)

@SchrodingersGat SchrodingersGat added CI CI / unit testing ecosystem dependency Relates to a project dependency labels May 24, 2024
@SchrodingersGat SchrodingersGat merged commit ed17179 into inventree:master May 24, 2024
24 checks passed
@SchrodingersGat
Copy link
Member

@zanieb looks like that has done the trick, many thanks :)

@SchrodingersGat SchrodingersGat added this to the 0.16.0 milestone May 24, 2024
@zanieb
Copy link
Contributor Author

zanieb commented May 24, 2024

Feel free to ping me if something breaks in the future.

@matmair
Copy link
Member

matmair commented May 24, 2024

Thank you for the PR @zanieb!

SchrodingersGat pushed a commit to SchrodingersGat/InvenTree that referenced this pull request May 24, 2024
SchrodingersGat added a commit that referenced this pull request May 24, 2024
* Merge pull request from GHSA-2crp-q9pc-457j

* ensure API login only works if mfa is not required

* add migration to log out users

* add migration to clear users

* Use `UV_SYSTEM_PYTHON` to allow the system Python interpreter instead of `VIRTUAL_ENV` (#7317)

* Fix docs links - pin to same branch

* Handle exception on migration

* Make migration non-atomic

---------

Co-authored-by: Matthias Mair <code@mjmair.com>
Co-authored-by: Zanie Blue <contact@zanie.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI / unit testing ecosystem dependency Relates to a project dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants