-
Notifications
You must be signed in to change notification settings - Fork 135
python: use 'uv add' when installing packages in certain scenarios #11154
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
base: main
Are you sure you want to change the base?
Conversation
|
E2E Tests 🚀 |
|
This is ready for review (or at least opinions on whether this is a good idea) but I put it in "draft" mode because I don't want to merge right before I go on holiday break. I'll probably end up merging after we cut 2026.01, so we can have all of January to test it out. I'll fix the failing tests when I get back, they just need more mocks. |
| const proxy = workspaceService.getConfiguration('http').get('proxy', ''); | ||
| if (proxy.length > 0) { | ||
| args.push('--proxy', proxy); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that uv doesn't support this --proxy argument, only pip does. I probably should make an issue and tackle that later.
|
|
||
| if (flags & ModuleInstallFlags.reInstall) { | ||
| args.push('--force-reinstall'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here - uv doesn't support this argument.
| const configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService); | ||
| const settings = configService.getSettings(isResource(resource) ? resource : undefined); | ||
| const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService); | ||
| const interpreter = isResource(resource) ? await interpreterService.getActiveInterpreter(resource) : resource; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh, I think getActiveInterpreter() still sometimes returns the wrong one in certain workspaceless scenarios. That's a bigger problem than this PR so I'll make an issue later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to #6936
Addresses #8870. Changes the
UVInstallerso it usesuv addinstead ofuv pip installifpyproject.tomlat the root--break-system-packagesflag, which isn't supported byuv add, and is only tried on the second iteration anyway in most casesThis will mainly be used by Assistant's install python package tool if you have uv installed, but also could be used in certain app workflows.
Should this be a setting?
Release Notes
New Features
uv addwhen appropriate when installing python packagesBug Fixes
QA Notes
Try
"python.useBundledIpykernel": falseand install ipykernel and make sure it didn't useuv add