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

Initialize remote extensions #16

Merged
merged 4 commits into from
Aug 24, 2022
Merged

Initialize remote extensions #16

merged 4 commits into from
Aug 24, 2022

Conversation

jeanp413
Copy link
Member

@jeanp413 jeanp413 commented Aug 22, 2022

Description

Initialize remote extensions with extensions from sync server

Related Issue(s)

Fixes gitpod-io/gitpod#7906
Related gitpod-io/openvscode-server#419

How to test

  1. Run this extension and connect to workspace
  2. Install this vsix gitpod-remote-ssh-0.0.39.zip so it's installed by default the second time
  3. Disconnect from workspace
  4. Enable settings sync
  5. Connect to workspace again and check sync extensions are installed in remote server

@mustard-mh
Copy link
Contributor

mustard-mh commented Aug 23, 2022

Code looks good 👍

But I got this error message when I'm testing it (image deleted)

Test works well

image

@akosyakov
Copy link
Member

akosyakov commented Aug 23, 2022

Please make sure to test other scenarios like not settings sync enabled, enabled with GH instead. How does it work then?

Also please consider backward compatibility with SH installations with older remote extensions, i.e. tests all scenarios against gitpod.io and SH last 2 releases (or at least consider it while looking at code) 🙏

private async initializeRemoteExtensions() {
let syncData: { ref: string; content: string } | undefined;
try {
syncData = await this.settingsSync.readResource(SyncResource.Extensions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder should not we rather provider gitpodHost here from connectionInfo instead of reading from settings. I think for remote window connectionInfo is source of the truth.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Settings sync config is generated from gitpod.host config not gitpodHost, ideally they are the same but it's possible they are not

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were discussing yesterday with self hosting team that having different configurations per host will be good, i.e. one should be able to specify different values for different hosts. Like use SSH Gateway for gitpod.io, but local app for SH solution and so on. How we designed it with one host does not really fit to it.

@mustard-mh
Copy link
Contributor

mustard-mh commented Aug 23, 2022

not settings sync enabled

It will ask to enable it (with notification and enable action)

enabled with GH instead

Same as above

cc @akosyakov

const action = 'Settings Sync: Enable Sign In with Gitpod';
const result = await vscode.window.showInformationMessage(`Couldn't initialize remote extensions, Settings Sync with Gitpod is required.`, action);
if (result === action) {
vscode.commands.executeCommand('gitpod.syncProvider.add');
}

@mustard-mh
Copy link
Contributor

Is it better to show the process bar when installing the extensions in remote? @jeanp413

}

const syncStoreURL = this.getServiceUrl();
const resourceURL = `${syncStoreURL}/v1/resource/${path}/latest`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeanp413
Copy link
Member Author

Added a notification with progress

@mustard-mh
Copy link
Contributor

mustard-mh commented Aug 24, 2022

It works well, and show logs really helpful 😄

Maybe we can also increase the timeout of requests to 5000ms? 1500ms is too short for me (today's network 🙈, failed two times with timed out)

@mustard-mh
Copy link
Contributor

Also wandering the release process, will we wait until __gitpod.initializeRemoteExtensions command is registered in backend?

@akosyakov
Copy link
Member

Should we enable it for all users? Can it break a user flow somehow? Should we go via experiment and analytics? i.e. land this PR only for prerelease for now, then start working on analytics and experiment? I would say at least without meaningful analytics we should not put in stable. cc @loujaybee

@jeanp413
Copy link
Member Author

jeanp413 commented Aug 24, 2022

Increased the timeout and added gitpod.remote.syncExtensions setting will be true by default in pre-release and false in stable

Can it break a user flow somehow?

This won't break user flow as it's an independent command

Should we enable it for all users?

I'd say it's safe to enable for all users, and with the added setting gitpod.remote.syncExtensions they can disable it if they want

For analytics only thing we can track is if the setting is true or false, we already send gitpod_desktop_settings_sync if settings sync is enabled or not

@jeanp413
Copy link
Member Author

Merging this and will publish a pre-release version

@jeanp413 jeanp413 merged commit d0aec88 into master Aug 24, 2022
@jeanp413 jeanp413 deleted the jp/init-remote-ext branch August 24, 2022 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants