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

Resolved Environments have an invalid type for workspaceFolder #20104

Closed
DonJayamanne opened this issue Oct 27, 2022 · 4 comments
Closed

Resolved Environments have an invalid type for workspaceFolder #20104

DonJayamanne opened this issue Oct 27, 2022 · 4 comments
Assignees
Labels
area-environments Features relating to handling interpreter environments author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug needs PR Ready to be worked on verified Verification succeeded
Milestone

Comments

@DonJayamanne
Copy link

When using the Python Extension API, the resolve Enviornments have an incorrect type for the property workspaceFolder.
workspaceFolder is not a Uri object, but a regular Object.

I've looked at the code, the problem is the fact that enviornments are serialized into JSON and then restored back again when storing from/into the memento.

Note: Objects with Uris cannot be stored in Memento nor can they be serialized into JSON and then parsed back

https:/DonJayamanne/pythonVSCode/blob/main/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts#L186

This is causing issues in the Python extension and new environments are not geting picked up because we're now filtering these enviornments based on the workspace Folder, however since the worksapceFolder isn't a Uri object the conditions fail and as a result new Python virtual envoirnments end up getting excluded in Jupyter extension.

@karthiknadig
Copy link
Member

@DonJayamanne this is my proposal to address this issue.

The Cause: The workspaceFolder field is set using an internal field searchLocation. searchLocation is what gets saved to Memento as Uri. when we read it back it looks like this:
image

Interim Solution:

  • Reading (broken case): Since there are already existing cases where this is broken. The surgical fix is to read bad value, extract the scheme and path, use that to re-create the URI.
  • Reading/Writing (new case): Make sure that we convert Uri to string before writing and parse back the Uri when reading.

Long term solution:
Object written to memento needs to be serialized using a custom serializer that validates all values before writing and after reading.

karrtikr pushed a commit that referenced this issue Oct 28, 2022
For #20104

The Cause: The `workspaceFolder` field is set using an internal field
`searchLocation`. `searchLocation` is what gets saved to Memento as
`Uri`. when we read it back it looks like this:

![image](https://user-images.githubusercontent.com/3840081/198420406-fb4c9b87-1276-4717-8043-69d95dcdba48.png)

Interim Solution:
* Reading (broken case): Since there are already existing cases where
this is broken. The surgical fix is to read bad value, extract the
`scheme` and `path`, use that to re-create the URI.
* Reading/Writing (new case): Make sure that we convert Uri to string
before writing and parse back the Uri when reading.

Long term solution:
Object written to memento needs to be serialized using a custom
serializer that validates all values before writing and after reading.
@karrtikr karrtikr added area-environments Features relating to handling interpreter environments needs PR Ready to be worked on and removed triage-needed Needs assignment to the proper sub-team labels Oct 28, 2022
@karrtikr karrtikr added this to the October 2022 milestone Oct 28, 2022
@karrtikr karrtikr added the author-verification-requested Issues potentially verifiable by issue author label Oct 28, 2022
@rzhao271
Copy link

What are some verification steps for this issue?
I was able to see my virtualenvs while selecting the kernel today, if that matters.

@karrtikr
Copy link

karrtikr commented Oct 28, 2022

I think if you're able to see all local environments this issue can be considered as verified 👍 Don can confirm.

@rebornix rebornix added verification-needed Verification of issue is requested and removed verification-needed Verification of issue is requested labels Oct 31, 2022
@DonJayamanne
Copy link
Author

We are no longer using the property workacpeFolders, and I can confirm the Python API now returns only workspace specific environments.

@DonJayamanne DonJayamanne added the verified Verification succeeded label Nov 1, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-environments Features relating to handling interpreter environments author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug needs PR Ready to be worked on verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants