-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Details on the A/B experiment removing python.pythonPath
support
#12313
Comments
I'd also like to add that this is a breaking change to my extension, which relies on the pythonPath setting. Possibly other extensions too. If the setting does change preferably an API would be provided to access the setting before it is removed. |
An API is exposed #11294. Follow all changes regarding pythonPath setting here #11015. #11294 (comment) describes how we helped upgrade code runner extension to use the new API, you can do similarly for your extension. This code snippet could be very helpful. @zor-el I recommend opening new issues for discussing how removing this setting is breaking things for you. |
@zor-el We are a large team of developers, and sharing |
This comment has been minimized.
This comment has been minimized.
@thernstig, perhaps the verbosity of my original comment is obscuring its essence (sorry if that's the case). My point was that settings are not transferable in general, not that they are not transferable at all, and that therefore a separation of transferable and non-transferable settings must clearly be the solution, not mere removal of settings. Obviously a definition about which settings are transferable and which aren't depends on the specific process of each team and cannot be universal. I can see how moving settings from
I guess my point on some deeper level is that pivotal decisions such as this don't have to be a zero sum game. The sharers of setting X don't have to lose in order for its non-sharers to win (and vice versa). There clearly are win-win scenarios here, we just need to move on from the either-or thinking. Which is why I'm trying to have a discussion broader than just my personal issues with the situation. |
I agree with @zor-el in that I think this is a drastic, breaking change that is not proportional to the magnitude of the problem at hand. I will restate what I think is the problem for clarity:
Firstly, I disagree with the practice that Ideally, a solution would be enabled by VS Code proper. An example would be to enable extending settings from other files (example proposal at microsoft/vscode#15909). Such a change would enable sharing some settings via version control while leaving the greater configuration up to the user. If such a change or something similar is enacted, it is desirable to then enact the idiomatic approach. Therefore any proposed solution should be able to pivot easily in the future. The proposed solution, as I understand it, is to store the python interpreter path in "internal storage" that is not directly visible to the user, require use of the vscode-python command
I see additional problems:
Moreover, there exists a low-effort solution to remedy pain in the meantime by keeping
Moreover, due to the generality of the approach, there are many other possible implementations and, most importantly, the power is left with the user.
Project-specific environment variables are little awkward, but once set up are straightforward to use. Another alternative, if settings do no change often, is to simply provide a settings template. In conclusion, the proposed solution is an over-engineered solution with multiple problems (obscurity of information, non-generality of solution, etc.) to a problem that shouldn't exist, and when it does there are proportionally straightforward work-arounds. If/when a more idiomatic approach is enabled by changes to VS Code, there will be inertia to walking back the proposed changes and the extension will be left in a state of awkwardness. |
Thanks, @williamcol3, for chiming in so profoundly, and for keeping it on a general level like I did. I'd like to reemphasize that, to me, it is not about weather or not sharing settings is a "good" thing or "bad", but about freedom of choice for everybody. (Sharing even a In that sense, I'd like to suggest a (hopefully) low friction solution alternative: Instead of storing those settings "internally" (this approach painfully reminds me of the Windows registry), how about keeping them in Thoughts? |
This comment has been minimized.
This comment has been minimized.
Thank you all for providing such a detailed feedback. Let me try to explain a few points:
Our intention is not to punish anyone. We're becoming a very popular extension so we need to be more careful with our settings and solutions. Sharing path settings could be a potential security concern (one of these #7805) so we decided to go for a route where users can still define default settings (in User scope) and then have that to be added by everyone in the team. We're also working on being better at detecting environments, so ideally users won't have to keep setting paths - they'll just be listed in the interpreter list once
The path to the selected interpreter is printed out in the output, so you can always look it up and save that information in your user settings.
In general this doesn't work super well anyway, since paths are OS specific. This only really works for environments that are located in your workspace (and that is more common for virtual environments), but then it doesn't work between Windows and macOS/Linux.
For choosing what you'd like to share or not, the cleanest solution would have to be provided by VS Code generally.
We are not going to revert to the python.pythonpath setting as is. Our plan is to deprecate the path settings in the workspace level - but we want to make sure we found a good solution before we implement it. Environment variables are tricky too since they raise even higher security concerns, so we just can't go that route. |
This comment has been minimized.
This comment has been minimized.
I generally agree that you could work around the problems listed above, but the point is that I fail to see how the proposed solution improves the current situation. I see a lot of negatives to the changes but not many positives.
Ideally, yes. But that isn't the only solution. For instance, the C/C++ extension has their own configuration file, a
You're saying you plan to deprecate the path settings at the workspace level? By implication, does this mean they are not deprecated at the user level? Path settings seem wholly more appropriate at the workspace level (which I suppose is what Select Interpreter is for(?)) when considering the generally recommended workflow for python projects (virtual environment per project).
Firstly, if an agent has enough access to place a malevolent python interpreter/environment on a user's machine, that agent has enough access to change environment variables and "internal storage" with equal ease. If the history of cyber security has taught us anything it is that obscurity is not a valid defense mechanism. Furthermore, malevolent changes to internal storage would be less visible to the user and thus arguably pose more of a security risk. Moreover, environment variables aren't a route that you, as extension authors, have to go. It is up to the users how they want to share settings. The point of the example was to show that there are viable, user-available solutions to the problem at hand (described in #2125). On that note, there really should be a single place that states:
It is difficult to keep track of the various parts across what feels like too many issues. Based on the above issue, it seems like the only problem being addressed is the awkwardness of sharing |
Side-note: I think some of you are underestimating how common it is to share workspace settings in a project. It is mentioned almost immediately in https://code.visualstudio.com/docs/getstarted/settings that explains these settings. Working with a team of 40+ developers, setting up all project-specific configuration by some passionate VS Code power-users saves a lot of time for everyone working in the project. We have found no downsides, it has saved us much headache. The That is why sharing interpreter settings (path) through that file is a bad idea - a virtual environment is host specific, user specific, OS specific. There is a highly upvoted feature (microsoft/vscode#40233) to enable user+workspace specific settings. I would not mind if the interpreter path in the future gets put there instead. |
I don't see anybody underestimating anything, least of all sharing settings. What we are against is moving the settings to some "internal" and inaccessible location, for reasons clearly stated above, and not honoring In our case, we use project template generators that are fairly adjusted to our specific needs (probably for similar reasons why your team might be sharing the workspace settings), and with the pythonPath setting completely gone, those templates will be partly broken. (They were a non-trivial investment, probably not unlike your settings.json template, so it's gonna hurt.)
In case where the virtual environment is in the workspace folder (as it commonly is), we often agree to name the venv dir uniformly and to share even pythonPath. There's no downside for us there. As stated previously, the problem obviously wasn't whether pythonPath can be set there (some users want that, some don't), but that 'Select Interpreter' puts it there without user approval. Thanks for the hint to microsoft/vscode#40233. It's very similar to what I've suggested above. I just upvoted it. :) Unfortunately that feature, if and when implemented, won't work with regard to our issues here, unless pythonPath (or a similar setting) is retained and placed in such a setting file. But that's something which apparently the extension team is adamantly opposed to, for reasons only hinted to but never disclosed. |
That is certainly my mistake. I assumed it would be recommended to not share due to how common it would be to have user+workspace specific settings. I would mark the general situation as a failure of VS Code and I anticipated it will be rectified eventually. Which I think furthers my point than any chosen solution should be easy to pivot to the future idiomatic solution.
Not just interpreter path but any path specification, possibly database connection information (vscode-database for example), and things like integrated terminal environment variables (which you can set in settings). This isn't just limited to interpreter path, which is one of the reasons I don't think |
It's hard to say how common it is to share workspace settings without seeing statistics. Personally I think it's a antipattern to commit workspace setting to github - different people have different computers and different extensions, there's no one perfect settings that fits everyone. I would assume most major projects have it gitignored, but I could be incorrect. |
@Almenon agree it needs statistics, my guess is as good as yours . But major projects do use it: https://github.com/microsoft/vscode/tree/master/.vscode Personally, we also felt a great upswing doing so. But I digress, this is side-step of the topic and I think we cleared this one out. I would recommend adding comments to microsoft/vscode#40233 on how you would like it to look.
We also do that 😄 We have our in the workspace folder and have forced everyone to install their venv into |
@Almenon At my workplace, we share the workplace folder because it contains everything needed for a project to get started (even extension recommendations). |
The following workaround will enable you to still share environments if,
If all the settings are empty, the extension autoselects the interpreter. The first place the extension looks in for the interpreter is the workspace folder itself, so the intended environment will automatically be selected. |
For now, no. The general solution has to come from VSCode microsoft/vscode#40233.
We do have a sequence of fallbacks we follow to select the interpreter,
|
Ok.
It would be really nice to be able to customize via settings this sequence (it ordering, potentially removing some entries and adding some custom entries to it). Wouldn't that be more acceptable to version control than current |
@jraygauthier Maybe, although that seems like much more work. Please take these discussions to #12665 @zor-el @williamcol3 I found one of the security concerns which motivated this solution #7805 for your reference. This was brought to us by the security team at Microsoft and independently by external people. |
Seems like the much easier solution would just be to get the users' permission to use a given path the first time the extension encounters it per workspace. I fail to see how storing the path and keeping it out of settings.json is necessary, but that is not a discussion for this thread. I still have not seen a benefit that the proposed solution offers that other solutions do not also provide. It does, however, at the very least, have a cost for transitioning. I believe that cost is unnecessary. |
@williamcol3 unfortunately prompting is a dark, black magic to get right. 😉 People will complain about prompting, and no matter if everyone here swore up and down that they personally would never complain about such a prompt, that doesn't mean other users would not feel put out by a prompting increase. There's also the simple fact that a lot of users just never pay attention to prompts, making the efficacy of them lower than you may expect. Now we obviously still use them to try to get people to pay attention, but this once again feeds back into some users just flat-out hating prompts and just perpetually ignore them. So it's just not as simple as tossing a prompt up and calling it a day, no matter how much we wished it was. And I will also say no one has denied that there isn't a cost to this change. But there is also a cost to not enacting a change like this as well. But we are continuing to talk about the situation on the team to see if we can come up with a solution that works for as many users as possible: those that Followupmicrosoft/vscode-eslint#1012 is about when eslint added a prompt requiring users to opt into running eslint via the user workspace and that has not been well received either. |
This comment has been minimized.
This comment has been minimized.
And on topic: I use VS code to work on projects where we use zc.buildout. It's common practice to have a virtualenv inside every project folder to have an isolated environment. We have even some tooling which can generate/modify the settings.json. I was never able (since 2019) to get VS code to pick up the correct virtualenv / venv Python automatically. That was until I started setting my pythonPath explicitly in the settings.json using:
The settings.json can be shared with other team members or put in VCS as we all have our Python inside the project and everything is relative/inside the workspace. zc.buildout also generates a list with paths of the used Python modules, which are written to an .env file. actually this is also called PYTHONPATH . In my settings.json I use python.envFile for that:
I agree with most other peoples feedback here: if you want to improve the automatic detection (which worked very poorly so far in my experience), do this by all means. But why remove a setting which is used actively by a significant group of users who scaffold the IDE settings using automation tools? Without any fallback? |
@fredvd I can totally relate to how you felt there. As a quick band aid though: I opted out of experiments and my pythonPath is now a valid option again, so everything works as it used to. I also turned off automatic updates so that I can keep a close eye on the changes to this extension before updating it. It's seems like more work, but it's less (and much less stress!) than trying to find out why stuff doesn't work that used to work perfectly. I hope this helps. |
I have the same feeling as lots of others in this thread, removing After reading all the comments above, following questions are still unclear to me:
|
Throwing in a use case description and some questionsI'm often working on one and the same projects on different computers with different paths, OS, user names. This is possible since every piece of setting is stored in human readable files (JSON) or SQLite DBs (editing JSON stored in SQLite is annoying, but still manageable). How that works:
There is a custom CLI tool run for the launch of VS Code (name it wrapper) and also for adapting everything to the current environment (OS, user, machine) when a workspace is being opened. I have a personal VS Code extension that knows its own "open workspace" command, offers a list of the configured When Code is launchedThen the tool iterates over everything of interest below
Finally, the CLI tool sets nice for Code to When a workspace is loadedThe tool touches any setting, debug or task config on workspace level and also iterates over all folder definitions in the file
SummaryThe big number of paths in
There are several threads/ posts on GitHub where people describe similar thinks. There are others wrapping VS Code to make it fit for the environment of request. QuestionsWhat happens when Will the current interpreter selection always be written to something an external tool can read from to get live information?
This hope to avoid that one will always see a prompt telling that the selected interpreter for a given project cannot be found just due to a switch from one computer to another. Will we see a prompt every first time we change to some new user account/ machine when working on a new project or even a new workspace definition ( I hope that will not happen because that would be disruptive and annoying. All best, |
As a general answer to people asking what our plans are, we are still planning to move forward with supporting the use-case of not using |
This comment about wanting to avoid prompting seems ironic given the way it's implemented is to show a prompt that sent me on a 1-2 hour diversion trying to work out what is going to break and why this change was being made, and it will presumably prompt almost every single user if/when it's merged in. Despite what I'm sure is best-intentions, to me this prompting change initially appeared hostile and aggressive; I don't remember consenting to this A/B testing and would never expect to be opted in to such experimental testing when using the VSCode stable builds. Your opt-out mechanism also has the exact same problem that you are purportedly trying to fix here, that there isn't a sub-workspace way to disable them. I'd suggest only deploying such major breaking-change testing to unstable installations in the future - the people who you can actually reasonably assume to have consented to experimental features. The FAQ and page I'm sent to barely covers anything - and neither of the tickets it sends you do seem to cover much discussion (which was apparently all private). The first ticket #2125 seems to propose "keep The questions I have (I realise some of which are touched on above) are:
I've had to try and dig through the internal storage before when I accidentally disabled a prompt I wanted to keep; and this was a nightmare, and it remains the case that VSCode really does have a lot of prompts that you need to pay attention to anyway. I've been a keen VSCode user for ~4 years, and recommend to people. Obviously I'm not going to stop using it but this A/B being forced into my face and public "all our discussions on this breaking change are private, you aren't allowed to know why" attitude really is the first thing that has even a little damaged my confidence. I'm opting out of this "feature" and all experiments now. |
This comment has been minimized.
This comment has been minimized.
@fredvd you mentioned the following:
We tried to make that clear in the wiki page we linked to in the notification prompt. What would have been more helpful, or at least have prevented you from spending a lot of time understanding what is going on? @jamilraichouni thanks for detailing your scenario. If @ndevenish This isn't an "experimental" change. We use the A/B experiment framework to gradually roll out changes instead of shipping to everyone as we normally would. We use that so we can get feedback and iterate on changes while providing a workaround to disable them (other than downgrading the extension and disabling auto-updates). We knew we had to make this change for security reasons, but we wanted to offer an explanation to those in the experiment as to what is going on and how to opt-out as we iterate on it. That was our intent with the wiki page. If there's more information we can add to that page that would help with answering questions, we’ll be happy to do so. As to your questions:
Because it doesn't solve the security concern that was brought up to us (e.g. #7805). Depending on the solution we deliver for this as mentioned by Brett, we may be able to have an opt-in mechanism for the python.pythonPath setting while still maintaining security protections, and then it could be used normally if microsoft/vscode#40233 is implemented.
Yes, it does as a workspace in VS Code is just the project root folder (not necessarily .code-workspace files, although it persists for those too).
Not impossible. You could add the path to the This may change though, as Brett mentioned in the comment above. We're not saying we will never offer an opt-in mechanism to share interpreter paths, we're just saying that it needs to be opt-in and not on by default.
Because the name is confusing given the PYTHONPATH env variable. E.g. #8646.
These are things to be considered in our plan but again as Brett mentioned, our solution needs to take into account technical implications and we need to get sign-off from our own security teams.
I explained the reason why we use the A/B framework for controlled rollout. As to the private discussions you mentioned, part of security concerns are exemplified in #7805, though we were specifically asked not to give more details so we don’t give attackers a heads-up on how to exploit users. |
Thanks for this detailed feedback! One question: Is there any true specification describing in detail where and how this new way of storing the interpreter selection in the local storage is happening/ working? VS Code is by far the most used tool on my side to do the daily business and also private stuff. For devs mostly doing Python development that setting is very central and of importance. I'd like to keep having a good understanding of what is happening under the hood. It is imaginable that others also want to see, if and how one can adapt automation tools to read and/ or touch this setting from outside (e. g. wrapper). Is that possible? Thank you, |
Yes, all features like this get a spec written by the team internally. Since the code is already committed, you can have a look yourself, but we are using VS Code's private store for extensions to store the path to the environment.
I am going to answer this by saying we are working on a potential spec and the required teams to be involved is growing, so we need to ask people to please be patient while we work through this. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
python.pythonPath
support
Hi everyone👋 We have replaced this experiment in favor of a new one which is slowly being rolled out. You can find the details in the updated wiki. In short,
Essentially Hopefully that'll work for most use cases. You can open new issues to provide feedback. |
https://devblogs.microsoft.com/python/python-in-visual-studio-code-july-2021-release/#selecting-a-python-interpreter-no-longer-modifies-workspace-settings announced how we resolved dropping |
Summary of the situation
For security reasons we need to not automatically trust the
python.pythonPath
setting when it originates from a.vscode/settings.json
file in a workspace. We also had a long-standing request to not write a user's environment path to.vscode/settings.json
to allow for it to be committed to version control without be specific to any one user's machine.We set up an A/B experiment to measure impact on users in dropping support for
python.pythonPath
. This issue is a response from users who were negatively impacted. We are working with various teams within Microsoft to try and come up with a situation where we can bring back some semblance of the old semantics while still providing the level of security and safety we need to.To stay updated on our work to resolve the situation, please feel free to subscribe to this issue -- there's a "Subscribe" button in the right column -- and we will post updates to this issue as things develop.
Related issues:
Wiki: https://github.com/microsoft/vscode-python/wiki/AB-Experiments
Opting out of the experimental semantics
To opt out of the experiment, you can set
“python.experiments.optOutFrom”
to either the specific experiment or to["All"]
to opt out of all experiments. See https://github.com/microsoft/vscode-python/wiki/AB-Experiments#faq for more details.Original post
Originally posted by @zor-el in #12002 (comment)
The text was updated successfully, but these errors were encountered: