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

Extensions not loaded the first time a remote workspace is trusted (when Azure ML ext is installed) #128573

Closed
sevillal opened this issue Jul 13, 2021 · 21 comments
Assignees
Labels
workspace-trust Trusted workspaces
Milestone

Comments

@sevillal
Copy link

Issue Type: Bug

  1. Install Azure Machine Learning and Azure Machine Learning - Remote extensions
  2. Connect to remote Compute Instance using Azure Machine Learning - Remote
  3. Open a python file and try to run it

Expected: Python extension gets loaded and you can run the file using pyhton extension

Failure: Python extension is not loaded and its commands are not found

Findings

  1. Azure Machine Learning - Remote require reload
    image
  2. Log: [2021-07-13 13:21:27.207] [renderer9] [error] ENOENT: no such file or directory, access 'c:/Users/sevillal/.vscode-insiders/extensions/ms-toolsai.vscode-ai-remote-0.3.0': Error: ENOENT: no such file or directory, access 'c:/Users/sevillal/.vscode-insiders/extensions/ms-toolsai.vscode-ai-remote-0.3.0'
  3. Reloading the window fixes the issue, python extension is loaded and its commands are available
  4. If "Azure Machine Learning - Remote" extension is installed in the remote target, and you connect with SSH - Remote, the same issue happens the first time a workspace is trusted

VS Code version: Code - Insiders 1.59.0-insider (807dfb8, 2021-07-13T09:24:20.727Z)
OS version: Windows_NT x64 10.0.19042
Restricted Mode: No
Remote OS version: Linux x64 4.15.0-1098-azure

System Info
Item Value
CPUs Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz (8 x 2112)
GPU Status 2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 15.92GB (0.73GB free)
Process Argv --crash-reporter-id 2b925b84-77da-4382-9d6c-89a345fc2762 --crash-reporter-id 2b925b84-77da-4382-9d6c-89a345fc2762 --crash-reporter-id 2b925b84-77da-4382-9d6c-89a345fc2762 --crash-reporter-id 2b925b84-77da-4382-9d6c-89a345fc2762 --crash-reporter-id 2b925b84-77da-4382-9d6c-89a345fc2762
Screen Reader no
VM 0%
Item Value
Remote Azure ML: sevillal-dev-ci
OS Linux x64 4.15.0-1098-azure
CPUs Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz (2 x 2397)
Memory (System) 6.81GB (2.40GB free)
VM 0%
Extensions (9)
Extension Author (truncated) Version
vscode-ai-remote ms- 0.3.0
remote-containers ms- 0.187.0
remote-ssh ms- 0.65.7
remote-ssh-edit ms- 0.65.7
python ms- 2021.6.944021595
vscode-pylance ms- 2021.7.2
jupyter ms- 2021.8.1025925145
vscode-ai-remote ms- 0.3.0
azure-account ms- 0.9.8
A/B Experiments
vsins829:30139715
vsliv368:30146709
vsreu685:30147344
vspor879:30202332
vspor708:30202333
vspor363:30204092
vspre833:30321513
vshan820:30294714
vscus158:30321503
vscod805cf:30301675
bridge0708:30335490

@roblourens
Copy link
Member

@sbatten @lszomoru why doesn't the Azure ML extension get activated right away here, I'm not sure what's different about it. Is it because the capabilities section isn't filled out? https://code.visualstudio.com/api/extension-guides/workspace-trust#onboarding You could see if that makes a difference @sevillal

@lszomoru lszomoru added the workspace-trust Trusted workspaces label Jul 14, 2021
@lszomoru
Copy link
Member

I was able to reproduce the issue and I will reach out to @sandy081 in order to track down the issue.

@lszomoru
Copy link
Member

@sandy081 and myself have looked at this and there indeed seems to be a problem when the Azure Machine Learning - Remote extension is being enabled as part of trusting a workspace. While we are working to track that down, we would like to suggest that you try the following:

  1. is there any reason why this extension defines both workspace and ui as extensionKind in the package.json? If there is no specific reason, we would like to suggest to only specify ui as the extensionKind in the package.json.
  2. since this extension implements a resolver, we would suggest that you enable support for untrusted workspaces as documented here: https://code.visualstudio.com/api/extension-guides/workspace-trust#onboarding

@lszomoru lszomoru added this to the August 2021 milestone Jul 26, 2021
@sevillal
Copy link
Author

sevillal commented Aug 4, 2021

@lszomoru,

  1. @shsuman can speak more about the need of [workspace, ui]
  2. Just tried the untrusted workspace config and that seems to fix the issue

@shsuman
Copy link
Member

shsuman commented Aug 5, 2021

Hey @lszomoru,
Azure ML extension has a hard dependency on the Azure ML - Remote extension because the remote connection feature although executed in the remote extension, is exposed in the main extension. Further, we also use the API exposed by the Remote extension in the main extension. Now since we have taken this hard dependency, we need the Remote extension and its API in the remote context as well.

@sbatten
Copy link
Member

sbatten commented Aug 5, 2021

Am I correct in understanding that the issue is resolved by adding untrusted workspace support to the package.json

@sevillal
Copy link
Author

sevillal commented Aug 6, 2021

@sbatten that's correct.

@sevillal
Copy link
Author

sevillal commented Aug 9, 2021

@lszomoru couple of questions?

  1. Is "enabling support for untrusted workspaces" the best practice for remote extensions? My hypothesis is that's because the remote extension knows the folder to land that should be "trusted".
  2. Is there any security implication we need to be aware of by "enabling support for untrusted workspaces"?

@alexdima
Copy link
Member

alexdima commented Aug 20, 2021

@sevillal Here is what you need to do in the package.json of ms-toolsai.vscode-ai-remote:

  • "extensionKind": ["ui", "workspace"]
  • "capabilities": { "untrustedWorkspaces": { "supported": true } } (because resolvers need to execute before workspace trust is established)
  • "api": "none" (because the main extension that needs to run on the workspace side takes a dependency on this extension which runs on the ui side). this blocks the extension from exposing any API.

The current version of vscode has a bug where your -remote extension will end up executing on both the ui and the workspace side when it is being used as a resolver. I plan to fix this bug, but it will be breaking.

@sevillal
Copy link
Author

@alexdima I tried the above changes but it fails when the main extension (ms-toolsai.vscode-ai) tries to use the api of the remote extension (ms-toolsai.vscode-ai-remote). I think the problem might be the "api": "none".

So our workflow would be:

  1. User in local context opens vscode and activates ms-toolsai.vscode-ai and ms-toolsai.vscode-ai-remote
  2. User starts remote connection
  3. ms-toolsai.vscode-ai extension uses ms-toolsai.vscode-ai-remote api to set some secrets used later by the resolver
  4. ms-toolsai.vscode-ai opens a remote uri (vscode-remote://...) that the ms-toolsai.vscode-ai-remote can resolve

@alexdima
Copy link
Member

alexdima commented Aug 23, 2021

@sevillal Thank you for the extra info. I am really sorry, but I am afraid you have ended up taking a dependency on a vscode bug. As it is right now, the ms-toolsai.vscode-ai-remote executes twice, once on the ui side and again on the workspace side. To clarify, running the same extension on two extension hosts at the same time has unforeseen consequences, like for example, commands are registered twice, views are registered twice, etc, and is completely untested, and unforeseen. The fact that anything works at all is more of a big coincidence, and IMHO we really need to fix this.

Here is what you can do to avoid having an api dependency:

  • continue having ms-toolsai.vscode-ai-remote as an extensionDependency for ms-toolsai.vscode-ai. This will ensure that the extension ms-toolsai.vscode-ai-remote is installed, enabled, and always activated before the extension ms-toolsai.vscode-ai (even if the two extensions execute on distinct extension hosts).
  • in ms-toolsai.vscode-ai-remote, use "api": "none". This forbids the extension from exporting API.
  • in ms-toolsai.vscode-ai-remote, register a command e.g. command1
  • from ms-toolsai.vscode-ai, invoke the command command1.

An alternative would be to change the extension kind of ms-toolsai.vscode-ai and make it a ui extension, but I don't know if your feature set would be compatible with running the extension on the ui side.

@alexdima
Copy link
Member

alexdima commented Sep 3, 2021

@sevillal Did you have a chance to try this?

@sevillal
Copy link
Author

sevillal commented Sep 16, 2021

@alexdima sorry I was off for some days.

I have tried your suggestion of using commands (instead of calling the extension API directly) and that seems to work.

I am curious about the following:

  1. Correct me if I am wrong but if we update to [ui] the extension does not need to be installed in remote and having it locally will be enough. Since "ms-toolsai.vscode-ai-remote" was [workspace, ui] we have users that have the extension installed in the remote target already, once we release the new version (with [ui] only) and the user tries to connect to thre remote, could this represent an issue?

@sevillal
Copy link
Author

sevillal commented Sep 17, 2021

@alexdima @lszomoru I was testing how other extensions behave with Remote SSH and found the following:

  1. Using Remote SSH connect to a target and open a folder
  2. Trusted workspace prompt appears, then select "Yes, I trust the authors"
  3. Open a python file and try to run it, nothing happens (no error is shown)
  4. If you go to extensions tab, the Python extension needs a reload

The issue we have faced with users is that they don't have any sort of indication that the reload is required until they go to the extensions tab.

Shouldn't VS Code show a notification that reload is required or reload after the workspace is trusted?

@alexdima
Copy link
Member

alexdima commented Sep 20, 2021

I have tried your suggestion of using commands (instead of calling the extension API directly) and that seems to work.

This is great! Please let me know when these changes are published, such that I can fix things on our side (better enforce that an extension can be at most activated once).

Correct me if I am wrong but if we update to [ui] the extension does not need to be installed in remote and having it locally will be enough. Since "ms-toolsai.vscode-ai-remote" was [workspace, ui] we have users that have the extension installed in the remote target already, once we release the new version (with [ui] only) and the user tries to connect to thre remote, could this represent an issue?

In this situation, the first time a user connects to such a remote, the user will have locally the ms-toolsai.vscode-ai-remote extension at a newer version with extension kind [ui]. When scanning the remote extensions, the old version of ms-toolsai.vscode-ai-remote will be probably hit, with the extension kind [workspace, ui]. From reading the code, I think the extension kind defined for the remote extension will "win" and overwrite the extension kind defined by the local extension. So for the first connection, the old version of the extension will activate remotely (as it does today). So the extension will be activated twice (as it does today). Once the remote extension is updated (should happen quickly after the first connection), subsequent connections will work as we want them. So, overall, yes, this is problematic, but I don't think it is a blocker.

@lszomoru ⬆️ Please see above comment

alexdima added a commit that referenced this issue Sep 23, 2021
…ionKind defined in the local extension over the remote extension in case they disagree (#128573)
@alexdima
Copy link
Member

@sevillal I have pushed a change in 2750398 where priority would be given to the extensionKind defined in the local extension, which has a higher chance of being up-to-date.

@sevillal
Copy link
Author

sevillal commented Sep 27, 2021

@lszomoru do you have any comment in regard to #128573 (comment)

cc @SiddhanthUnnithan

@lszomoru lszomoru modified the milestones: September 2021, October 2021 Sep 28, 2021
@lszomoru
Copy link
Member

@sevillal, sorry for the delay in getting back to you on this. Trusting a workspace should enable all extensions that were disabled due to their workspace trust requirement without the need to reload the window. I will take a look at this first thing tomorrow morning and update the issue with my findings.

@lszomoru
Copy link
Member

@sevillal, I have tried to reproduce the behaviour that you have described with the latest insiders release without any luck. Could you please ping me when you get online in the morning so that we get a screen sharing session going so that I can take a closer look at your setup? Thank you!

@sevillal
Copy link
Author

@lszomoru I was testing in latest stable, sorry about that. I tested latest insider and seems to be working fine. Is there any tentative date for the next vscode release with this fix? Thanks!

@lszomoru
Copy link
Member

We are wrapping up the September endgame this week, so we expect the next Stable release to happen in the second half of next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
workspace-trust Trusted workspaces
Projects
None yet
Development

No branches or pull requests

7 participants