Skip to content
This repository has been archived by the owner on Jul 19, 2019. It is now read-only.

CHE-9265: Implementation Theia terminal plugin. #1

Merged
merged 23 commits into from
Jul 5, 2018

Conversation

AndrienkoAleksandr
Copy link
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented Apr 25, 2018

Implementation Theia terminal plugin

This pull request implements server and client side for creation multi-machine terminals in the Eclipse CHE workspaces(for docker environment). Client side and server side communicate with help websocket connections(protocol json-rpc). To manage all terminals used separated json-rpc connection, one per client. To send input/output of the terminal uses separated simple websocket connection.

Terminal Server

Server side was written on the go-lang and uses docker cli to create terminal connections. Terminal connection based on docker exec.
Server side supported:

  • Create new terminal exec;
  • Connect to the created terminal exec;
  • Resize terminal exec;
  • Ability to reconnect to the created terminal exec with restore content. Restore content implemented like ring buffer with size 1000 lines.
    Kill terminal doesn't supported yet, because docker api doesn't support kill exec. See more: kill exec proposal
    For dependencies management used dep tool https://github.com/golang/dep.

Terminal client

For now client side it's Theia Extension based on original Theia terminal extension, but modificated to support connection to the external server side. And this extension support ability select machine to terminal creation from command palette machine list. Minimal Theia core 0.3.8.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
…binary inside docker container.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Remove development changes.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
… docker cli with version v17.03.2-ce(api version 1.26). Set up minimal api version 1.25 to compability with docker 1.13.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
…dockerfiles.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Contributor Author

AndrienkoAleksandr commented Apr 25, 2018

Don't review "vendor" directory it's external dependencies controlled by dep tool.

@AndrienkoAleksandr
Copy link
Contributor Author

Creation CQ's in progress.

@AndrienkoAleksandr
Copy link
Contributor Author

Needed eclipse-che/che#9475

@benoitf
Copy link
Member

benoitf commented Apr 25, 2018

Don't review "vendor" directory it's external dependencies controlled by dep tool.

is this directory has to be checked in in the repository ? https://golang.github.io/dep/docs/FAQ.html#should-i-commit-my-vendor-directory

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Contributor Author

I think, we can save all used dependencies to have stable reproducible build protected with changes history of upstream projects. And for our go-lang agents we saved sources of external dependencies in the vendor folder, but we did it with help Godeps(for now this project is archived and recommends to use dep tool). We skips test tool and unused packages form upstream projects:
[prune]
go-tests = true
unused-packages = true

@skabashnyuk
Copy link

How is this feature is going to work in OpenShift/Kubernetes environments?

@AndrienkoAleksandr AndrienkoAleksandr changed the title Implementation Theia terminal plugin. CHE-9265:Implementation Theia terminal plugin. Apr 26, 2018
@AndrienkoAleksandr AndrienkoAleksandr changed the title CHE-9265:Implementation Theia terminal plugin. CHE-9265: Implementation Theia terminal plugin. Apr 26, 2018
@ghost
Copy link

ghost commented Apr 26, 2018

@skabashnyuk it won't.

@AndrienkoAleksandr
Copy link
Contributor Author

CQ for docker(moby) cli https://dev.eclipse.org/ipzilla/show_bug.cgi?id=16187


private async doResize() {
await Promise.all([this.waitForResized.promise, this.waitForTermOpened.promise]);
const geo = (this.term as any).proposeGeometry();
Copy link
Member

Choose a reason for hiding this comment

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

term's type is Xterm.Terminal. Is there any reason to use type assertion to any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch.

this.cols = size.cols;
this.rows = size.rows;

let resizeParam: ResizeParam = {
Copy link
Member

Choose a reason for hiding this comment

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

A lot of variables within the PR are declared as let but they're actually constants. Should they be declared as const?

Copy link
Contributor Author

@AndrienkoAleksandr AndrienkoAleksandr Apr 27, 2018

Choose a reason for hiding this comment

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

Yes, it should be. Done.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Contributor Author

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@AndrienkoAleksandr
Copy link
Contributor Author

CQ xterm.js 3.3.0 https://dev.eclipse.org/ipzilla/show_bug.cgi?id=16190 approved.

@AndrienkoAleksandr
Copy link
Contributor Author

awaiting for CQ for docker(moby) cli.

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.

4 participants