remote: first cut at 'inline' remote resolvers #180263
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For web, it seems the most feasible direction for resolvers as we make existing remote extensions 'web enabled' is to allow them to run in the extension host. However, in no case will there just be a simple websocket we can connect to ordinarily. This PR implements a first cut at 'inline' resolvers where messaging is done in the extension host. I have not yet tested them on web, where I think some more wiring is needed to mirror desktop. Also, resolution of URLs is not in yet. I think for this we'd want to do some service-worker -based 'loopback' approach to run requests inline in the remote connection, similar to what I did for tunnels... Resolvers are not yet run in a dedicated extension host, but I think that should happen, at least on web where resolvers will always(?) be 'inline'. Most of the actual changes are genericizing places where we specified the "host" and "port" previously into an enum. Additionally, instead of having a single ISocketFactory, there's now a collection of them, which the extension host manager registers into when a managed resolution happens.
connor4312
force-pushed
the
connor4312/inline-remote-resolver
branch
from
3304b06
to
f5427ee
Compare
alexdima
reviewed
May 9, 2023
src/vs/platform/remote/electron-sandbox/remoteAuthorityResolverService.ts
Outdated
Show resolved
Hide resolved
…e-remote-resolver
…rity doesn't contain a '+' and is not in a 'hostname:port' format
…and extHostManagedSockets)
alexdima
reviewed
May 11, 2023
src/vs/workbench/services/environment/electron-sandbox/environmentService.ts
Outdated
Show resolved
Hide resolved
…r' into connor4312/inline-remote-resolver
|
@connor4312 Because I pushed code to the PR, it will not get auto-merged. We need one more approver. |
|
Yes, I posted in #codereview, I think someone mis-✅'d it 🤔 |
aeschli
approved these changes
May 15, 2023
|
There's a regression from this PR, see #183901 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For web, it seems the most feasible direction for resolvers as we make
existing remote extensions 'web enabled' is to allow them to run in the
extension host. However, in no case will there just be a simple
websocket we can connect to ordinarily.
This PR implements a first cut at 'inline' resolvers where messaging is
done in the extension host. I have not yet tested them on web, where I
think some more wiring is needed to mirror desktop. Also, resolution of
URLs is not in yet. I think for this we'd want to do some service-worker
-based 'loopback' approach to run requests inline in the remote
connection, similar to what I did for tunnels...
Resolvers are not yet run in a dedicated extension host, but I think
that should happen, at least on web where resolvers
will always(?) be 'inline'.
Most of the actual changes are genericizing places where we specified
the "host" and "port" previously into an union. Additionally, instead of
having a single ISocketFactory, there's now a collection of them, which
the extension host manager registers into when a managed resolution
happens.