Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Modify 'configStorage' path for remote plugin #488

Merged
merged 1 commit into from
Oct 16, 2019

Conversation

evidolob
Copy link
Contributor

What does this PR do?

Modify 'configStorage' path for remote plugin, as /home/theia/ may not present in remote plugin image

What issues does this PR fix or reference?

eclipse-che/che#14797

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
// Modify 'configStorage' objects path, to use current user home directory
// in remote plugin image '/home/theia' doesn't exist
const originalStart = PluginManagerExtImpl.prototype.$start;
PluginManagerExtImpl.prototype.$start = async function (params: PluginManagerStartParams): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a hack: if the storage path is not in fact determined from the parameters, we should refactor the plugin API to make the storage path the responsiblity of the plugin runner.
I understand that we're trying to get this in urgently, so we do a solution in che-theia, but could we open an issue on the theia repo to do the refactoring?

@che-bot
Copy link
Contributor

che-bot commented Oct 15, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

if required for 7.3.0 let's merge it, else as Thomas said, we should make changes upstream

@evidolob
Copy link
Contributor Author

evidolob commented Oct 16, 2019

Yes, it required for 7.3.0, I'm going to create issue in Theia repository: eclipse-theia/theia#6395

@evidolob evidolob merged commit 0050509 into master Oct 16, 2019
@evidolob evidolob deleted the modify-remote-plugin-configStorage branch October 16, 2019 07:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants