Custom socket provider and java language server in sidecar by JPinkney · Pull Request #9616 · eclipse-che/che

JPinkney · 2018-05-04T13:57:49Z

Signed-off-by: jpinkney josh.pinkney@mail.utoronto.ca

What does this PR do?

Adds the ability to run jdt.ls in a sidecar and allows custom socket launchers.

What issues does this PR fix or reference?

#8874

codenvy-ci · 2018-05-04T13:59:02Z

Can one of the admins verify this patch?

codenvy-ci · 2018-05-04T13:59:06Z

Can one of the admins verify this patch?

codenvy-ci · 2018-05-04T14:01:03Z

Can one of the admins verify this patch?

ghost · 2018-05-07T05:55:14Z

.../src/main/java/org/eclipse/che/api/languageserver/remote/CustomSocketLsLauncherProvider.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.


@JPinkney do we really need this? Maybe I am missing smth, but reading ws config and grabbing servers of a special type has been already implemented by @dkuleshov https://github.com/eclipse/che/blob/master/wsagent/che-core-api-languageserver/src/main/java/org/eclipse/che/api/languageserver/remote/SocketLsLauncherProvider.java Can't we just reuse it?

skabashnyuk · 2018-05-07T06:07:28Z

multiuser/api/che-multiuser-api-workspace-activity/pom.xml

        </dependency>
+        <dependency>
+            <groupId>org.eclipse.che.multiuser</groupId>
+            <artifactId>che-multiuser-api-resource</artifactId>


why do you need this artifact?

skabashnyuk · 2018-05-07T06:08:13Z

...c/main/java/org/eclipse/che/plugin/java/languageserver/JavaSocketLanguageServerLauncher.java

+import org.eclipse.lsp4j.services.LanguageServer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+


skabashnyuk · 2018-05-07T06:09:02Z

...er/src/main/java/org/eclipse/che/api/languageserver/remote/SocketLanguageServerLauncher.java

+import org.eclipse.lsp4j.jsonrpc.Launcher;
+import org.eclipse.lsp4j.services.LanguageClient;
+import org.eclipse.lsp4j.services.LanguageServer;
+


skabashnyuk · 2018-05-07T06:09:57Z

...er/src/main/java/org/eclipse/che/api/languageserver/remote/SocketLanguageServerLauncher.java

+  public LanguageServer launch(String projectPath, LanguageClient client)
+      throws LanguageServerException {
+    try {
+      Socket socket = new Socket(host, port);


are you going to close this Socket on @PreDestroy?

ghost · 2018-05-08T07:11:49Z

@JPinkney can you write a short how-to with what one can do to test this PR? I have an image with jdt.ls starting in tcp mode.

Also, why does the server need to have attributes - serverAttributes.get("custom_launcher").equals("true"))? Right now, it is enought to have "type": "lsp" in server attributes in order for this server to be counted as a remote language server.

tolusha · 2018-05-08T10:59:03Z

...server/src/main/java/org/eclipse/che/api/languageserver/remote/SocketLsLauncherProvider.java

-    }
+          boolean foundCustomLauncher = false;
+          if (serverAttributes.get("custom_launcher") != null
+              && serverAttributes.get("custom_launcher").equals("true")) {


It is better to use here equalsIgnoreCase

tolusha · 2018-05-08T11:49:02Z

...c/main/java/org/eclipse/che/plugin/java/languageserver/JavaSocketLanguageServerLauncher.java

+  public LanguageServer launch(String projectPath, LanguageClient client)
+      throws LanguageServerException {
+    try {
+      Socket socket = new Socket(host, port);


Should we close this socket later?

tolusha · 2018-05-08T11:50:51Z

ide/che-core-ide-stacks/src/main/resources/stacks.json

    }
  }
-]
+]


Don't forget to add empty line.

JPinkney · 2018-05-08T19:17:32Z

To test the instructions are almost the same as the regular sidecar instructions with a few exceptions.

1. Change the custom recipe to include che-jdt-ls/java-ls as the image (or your own image)
2. Inside the servers object in the java machine set the id in the workspace config as "org.eclipse.che.plugin.java.languageserver",  port as 5555, and languageID as java
3. Mount /home/user/jdtls as a volume for the java-machine-ls

ghost · 2018-05-08T19:25:06Z

Set the id in the workspace config as "org.eclipse.che.plugin.java.languageserver", port as 5555, and languageID as java

Can you share an example? Where exactly in a workspace config should I provide it?

JPinkney · 2018-05-08T19:59:05Z

Inside the server for the java machine like this

"servers": {
  "ls": {
    "attributes": {
      "internal": "true",
      "type": "ls",
      "config": "{\"id\":\"org.eclipse.che.plugin.java.languageserver\", \"languageIds\":[java]}"
    },
    "protocol": "tcp",
    "port": "5555"
  }
}

ghost · 2018-05-08T20:02:08Z

So, server attributes :)

Thanks, I will give it a try,

tsmaeder · 2018-05-11T09:07:41Z

...c/main/java/org/eclipse/che/plugin/java/languageserver/JavaSocketLanguageServerLauncher.java

+    try {
+      socket.close();
+    } catch (IOException e) {
+


Eating exceptions should only happen if you know exactly what is happening. In this case, you eat it because you say we don't care, but someone else might care, so please log it as a warning

tsmaeder · 2018-05-11T09:17:47Z

...c/main/java/org/eclipse/che/plugin/java/languageserver/JavaSocketLanguageServerLauncher.java

+
+  @Override
+  public LanguageServer launch(String projectPath, LanguageClient client)
+      throws LanguageServerException {


If I understand the code correctly, we have to do a whole lot of work here (managing the socket, etc.), only because we cannot pass in the parameter "JavaLanguageServer.class" into the super call. Why don't we extend the launcher interface like so?

<T extends LanguageServer> T launch(String fileUri, Class<T> serverClass, LanguageClient client) throws LanguageServerException;

Then we could simply wrap the super call and the class would be much simpler.

I've updated the PR to have this. However, instead of having Class serverClass I pass in the serverClass via the constructor of SocketLanguageServerLauncher. AFAIK launch only happens in LanguageServerRegistryImpl and we aren't able to pass in a serverClass without changing every single launch method for all the language servers and also we wouldn't necessarily know which serverClass to pass into the launch.

tsmaeder · 2018-05-11T09:19:38Z

...er/src/main/java/org/eclipse/che/api/languageserver/registry/LanguageServerRegistryImpl.java

                fileUri);
            String rootUri = launchingStrategy.getRootUri(fileUri);
-            InitializeParams initializeParams = prepareInitializeParams(rootUri);
+            InitializeParams initializeParams = prepareInitializeParams(launcher, rootUri);


I would just pass in a boolean "isLocal" here. It reduces the interface exposed.

codenvy-ci · 2018-07-05T09:49:04Z

Can one of the admins verify this patch?

skabashnyuk · 2018-07-05T11:27:56Z

@JPinkney this pr looks strange. It has too many changes. Can you rebase and squash as single commit?

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

JPinkney requested review from ashumilova, dkulieshov, evidolob and tolusha as code owners May 4, 2018 13:57

tolusha force-pushed the 5730_java_ls_poc branch from d1c3957 to 8fbf0e6 Compare May 4, 2018 14:28

JPinkney force-pushed the jdt.ls_sidecar branch from ce50de3 to 97d2e48 Compare May 4, 2018 14:53

JPinkney requested a review from vparfonov as a code owner May 4, 2018 15:29

ghost reviewed May 7, 2018

View reviewed changes

skabashnyuk reviewed May 7, 2018

View reviewed changes

JPinkney force-pushed the jdt.ls_sidecar branch 2 times, most recently from 498d098 to 2d5af5e Compare May 7, 2018 17:22

tolusha reviewed May 8, 2018

View reviewed changes

JPinkney force-pushed the jdt.ls_sidecar branch from 2d5af5e to e102cb9 Compare May 8, 2018 14:49

JPinkney mentioned this pull request May 9, 2018

Updated java images to have .m2 folder and jdtls data folder eclipse-che/che-dockerfiles#182

Merged

tsmaeder added the team/languages label May 11, 2018

tsmaeder requested changes May 11, 2018

View reviewed changes

tolusha force-pushed the 5730_java_ls_poc branch from 245c161 to 62ecb10 Compare May 12, 2018 19:47

JPinkney force-pushed the jdt.ls_sidecar branch from 699ba50 to b3edb6f Compare June 18, 2018 04:18

tsmaeder mentioned this pull request Jun 18, 2018

Languages Sprint 151 #10075

Closed

26 tasks

tsmaeder approved these changes Jun 20, 2018

View reviewed changes

tsmaeder force-pushed the 5730_java_ls_poc branch from 9d19fa1 to 6e9e9c4 Compare June 26, 2018 13:06

tsmaeder mentioned this pull request Jul 5, 2018

Languages Sprint 152 #10299

Closed

17 tasks

JPinkney force-pushed the jdt.ls_sidecar branch from b3edb6f to 9d55bc0 Compare July 6, 2018 12:36

Fixed java language server in sidecar

e4f9139

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

JPinkney force-pushed the jdt.ls_sidecar branch from 9d55bc0 to e4f9139 Compare July 6, 2018 12:37

tsmaeder merged commit f31ff0e into eclipse-che:5730_java_ls_poc Jul 6, 2018

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jul 6, 2018

tsmaeder pushed a commit that referenced this pull request Jul 13, 2018

Fixed java language server in sidecar (#9616)

e6d57a0

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Jul 27, 2018

Fixed java language server in sidecar (#9616)

e1995da

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tolusha pushed a commit that referenced this pull request Aug 7, 2018

Fixed java language server in sidecar (#9616)

b81d296

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Aug 22, 2018

Fixed java language server in sidecar (#9616)

c4f3215

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Aug 29, 2018

Fixed java language server in sidecar (#9616)

1d194e5

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tolusha pushed a commit that referenced this pull request Sep 5, 2018

Fixed java language server in sidecar (#9616)

29990f7

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Sep 13, 2018

Fixed java language server in sidecar (#9616)

aedf4a1

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Sep 20, 2018

Fixed java language server in sidecar (#9616)

5278226

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Sep 26, 2018

Fixed java language server in sidecar (#9616)

0aa9a11

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 1, 2018

Fixed java language server in sidecar (#9616)

8fb1ebf

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 5, 2018

Fixed java language server in sidecar (#9616)

9941e43

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 12, 2018

Fixed java language server in sidecar (#9616)

be521f2

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 16, 2018

Fixed java language server in sidecar (#9616)

f89997e

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

Fixed java language server in sidecar (#9616)

21d16c2

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

Fixed java language server in sidecar (#9616)

a54c378

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

Fixed java language server in sidecar (#9616)

ef28f58

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

Fixed java language server in sidecar (#9616)

89f0d44

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

Fixed java language server in sidecar (#9616)

bbaa4d7

Signed-off-by: jpinkney <josh.pinkney@mail.utoronto.ca>

                   }
                 }
-              ]
+              ]

                
                    No newline at end of file

Conversation

JPinkney commented May 4, 2018

What does this PR do?

What issues does this PR fix or reference?

Uh oh!

codenvy-ci commented May 4, 2018

Uh oh!

codenvy-ci commented May 4, 2018

Uh oh!

codenvy-ci commented May 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented May 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JPinkney commented May 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented May 8, 2018

Uh oh!

JPinkney commented May 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented May 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codenvy-ci commented Jul 5, 2018

Uh oh!

skabashnyuk commented Jul 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JPinkney commented May 8, 2018 •

edited

Loading

JPinkney commented May 8, 2018 •

edited

Loading