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

Replace deasyncPromise with async #1112

Closed
wants to merge 1 commit into from
Closed

Replace deasyncPromise with async #1112

wants to merge 1 commit into from

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented May 20, 2021

Signed-off-by: Igor Vinokur [email protected]

What does this PR do?

Replace deasyncPromise with async logic because of the issue with the deasync package.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes eclipse-che/che#19686

How to test this PR?

See eclipse-che/che#19686

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

@@ -208,7 +207,7 @@ export class PluginRemoteNodeImpl implements PluginRemoteNode {
}

// remote call for this method
return deasyncPromise(remoteBrowser.$callMethod(hostId, pluginId, callId, proxyDefinition.name, ...args));
return remoteBrowser.$callMethod(hostId, pluginId, callId, proxyDefinition.name, ...args);
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK it's not a proper fix as it'll break all other extensions that expect to have synchronous response on some calls.

Removing deasync is removing an expected feature

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.

It's removing some feature of #645

I think that for example vscode-yaml and kubernetes plug-in will no longer work as separate sidecars

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #1112 (02ba1d4) into master (eeb4526) will increase coverage by 2.78%.
The diff coverage is 54.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1112      +/-   ##
==========================================
+ Coverage   29.45%   32.23%   +2.78%     
==========================================
  Files         277      281       +4     
  Lines        9336     9545     +209     
  Branches     1380     1433      +53     
==========================================
+ Hits         2750     3077     +327     
+ Misses       6487     6372     -115     
+ Partials       99       96       -3     
Impacted Files Coverage Δ
...theia-about/src/browser/about-che-theia-dialog.tsx 0.00% <ø> (ø)
...ovisioner/src/node/git-configuration-controller.ts 0.00% <0.00%> (ø)
...eia-plugin-remote/src/node/hosted-plugin-remote.ts 0.00% <0.00%> (ø)
...theia-plugin-remote/src/node/plugin-host-custom.ts 0.00% <0.00%> (ø)
...in-remote/src/node/plugin-remote-backend-module.ts 0.00% <0.00%> (ø)
...theia-plugin-remote/src/node/plugin-remote-init.ts 0.00% <0.00%> (ø)
...-plugin-remote/src/node/plugin-remote-node-impl.ts 0.00% <0.00%> (ø)
...-che-theia-plugin-remote/src/node/plugin-remote.ts 0.00% <0.00%> (ø)
...ugin-remote/src/node/server-plugin-proxy-runner.ts 0.00% <0.00%> (ø)
...plugin-remote/src/node/terminal-container-aware.ts 0.00% <0.00%> (ø)
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7fba8f...02ba1d4. Read the comment docs.

@che-bot
Copy link
Contributor

che-bot commented May 20, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia quay.io/crw_pr/che-theia:1112
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1112

Tested with Eclipse Che on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@vinokurig
Copy link
Contributor Author

@benoitf

I think that for example vscode-yaml and kubernetes plug-in will no longer work as separate sidecars

Since the deasync is broken I wonder if this feature works now?

@vinokurig vinokurig closed this Jun 11, 2021
@vinokurig vinokurig deleted the che-19686 branch November 19, 2021 07:28
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.

Plugins can't communicate properly if they are located in different sidecars
3 participants