Integrate Python Language Server to Monaco Editor (Closes #1908, #571) #1910

umesh-timalsina · 2020-09-17T21:51:42Z

To run this PR locally. Checkout the branch:

Do a docker build for the language server json file:

$ docker build -t langServers:latest -f docker/Dockerfile.langservers .

Update components.json to add the following key:

"LanguageServers" : {
  "hostName": "http://localhost:5000",
  "servers": {
    "python": {
      "command": "Microsoft.Python.LanguageServer",
      "init": {
        "interpreter": {
          "properties": {
            "InterpreterPath": "/opt/conda/bin/python3"
          },
          "code": "import func",
          "item": "functools"
        }
      },
      "workspace": "/tmp/python-models/"
    }
  }
}

Spin up the container for language servers:

$ docker run -p 5000:5000 -it langServers:latest

Run npm start:

$ npm start

brollb

Nice work. I read through the code and provided some feedback. I still have to test it more thoroughly but figured I would provide my initial feedback first and then post any other comments after playing with it more.

environment.server.yml

brollb · 2020-09-23T16:40:13Z

src/visualizers/widgets/TextEditor/TextEditorWidget.js

+    TextEditorWidget.prototype._initializeLanguageClient = function () {
+        this.languageClient = new DeepforgeLanguageClient(
+            this.editor,
+            `${LANGAUGE_SERVER_HOST}/${this.language}`,


LANGAUGE -> LANGUAGE

brollb · 2020-09-23T16:42:08Z

config/config.extensions.js

+    config.extensions.languageServers = {
+        host: process.env.DEEPFORGE_LANGUAGE_SERVER_HOST,
+        servers: getAvailableLanguageServers(),
+        workspaceURIs: getWorkspaceURIs(),


After looking at how this is used, maybe this should be called rootURIs or rootWorkspaceURIs? When I first read it, I was concerned that all users may be sharing the same workspace (which I don't think will be an issue).

brollb · 2020-09-23T16:42:21Z

environment.worker.yml

@@ -6,3 +6,4 @@ dependencies:
      - matplotlib==3.2.2
      - simplejson
      - plotly
+      - python-language-server==0.25.0


Same comment as above.

brollb · 2020-09-23T16:47:41Z

src/visualizers/widgets/TextEditor/DeepForgeLanguageClient.js

+    './lib/reconnecting-websocket.min',
+], function (
+    vscodeWSJSONRpc,
+    LangaugeClient,


Langauge -> Language

My bad on the typos. Sorry.

brollb · 2020-09-23T16:53:04Z

config/config.extensions.js

+
+const SERVERS_YML = path.join(__dirname, '..', 'language-servers.yml');
+
+function getAvailableLanguageServers () {


I am a little wary about this. It seems to be assuming that any servers defined in language-servers.yml are available but since that file is checked in to git, it seems likely that this will end up with invalid language servers.

Yep. can it be solved by moving language-servers.yml to the deployment directory and using the config in production mode only?

brollb · 2020-09-23T16:55:47Z

src/visualizers/widgets/TextEditor/TextEditorWidget.js

@@ -23,6 +27,18 @@ define([
    MonacoLanguages = JSON.parse(MonacoLanguages);

    const AVAILABLE_KEYBINDINGS = ['default', 'vim'];
+    const LANGAUGE_SERVER_HOST = gmeConfig.extensions.languageServers.host || getDefaultServerURL();


I am not sure that we should have a default hard coded here. I was imagining that, if no language server is specified in the config, it will run without any language servers enabled (rather than assuming it is running on port 5000). Maybe this is what you were referring to when we were talking earlier. If so, my bad - I didn't realize you were referring to hardcoding the port here.

Since language servers are not required for use, it should proceed without one if none are specified.

Agreed will change it to not use language servers if no host name available.

umesh-timalsina · 2020-09-25T17:43:45Z

I have changed this PR to use microsoft's python language server. This still needs some changes to the TextEditorWidget code.

…ws-proxy; minor fixes to the TextEditorWidget

…eClient

brollb

A couple little comments. I am spinning it up now and will comment with any other issues I encounter!

brollb · 2020-10-02T16:58:08Z

.deployment/README.md

@@ -5,9 +5,13 @@ The script `deploy-deepforge` is used for standard deployment of deepforge using

 Additionally, this contains a file with customizations to the standard docker-compose.yml file which allows us to modify the entrypoint and install a version of tensorflow [compatible with the CPU of the deployment machine](https://github.com/deepforge-dev/deepforge/issues/1561).

-The deployment is updated by first creating the custom docker compose file using [yaml-merge](https://github.com/alexlafroscia/yaml-merge):
+Moreover, we also a proxy server with that spins up different language servers that we provide for
+ intelligent syntax


This looks like some odd line breaking...

brollb · 2020-10-02T16:58:50Z

.deployment/README.md

-The deployment is updated by first creating the custom docker compose file using [yaml-merge](https://github.com/alexlafroscia/yaml-merge):
+Moreover, we also a proxy server with that spins up different language servers that we provide for
+ intelligent syntax
+ highlighting in deepforge's browser. For more information checkout the language server's docker [file](../docker/Dockerfile.langservers)


"in deepforge's browser" -> "in DeepForge"

environment.server.yml

brollb · 2020-10-02T17:19:06Z

utils/languageServers.json

@@ -0,0 +1,18 @@
+{


Should this be in utils? It seems deployment specific

(and only used by docker files)

Yeah. This could be moved to .deployment but the problem is .deployment is in our .dockerignore. Not sure where to put this file.

brollb · 2020-10-02T21:48:44Z

Hmm... I am seeing some errors when trying to build the docker image:

…-pyls-integration

…uageServers (test)

brollb · 2021-01-08T15:45:39Z

Have you seen this error before while building the docker container?

umesh-timalsina · 2021-01-08T15:47:11Z

No, I have not, but I will have a look.

umesh-timalsina · 2021-01-20T19:27:51Z

I think the issue had to do with js-yaml safeDump function being removed from the latest release. I have fixed it. I have also deployed the branch at dev.deepforge.org. Should be good to go for testing in few minutes.

brollb · 2021-01-20T20:10:19Z

Nice. That fixed it for me - thanks! Now I can check out the rest of it :)

brollb · 2021-01-21T18:43:37Z

Overall, this looks good. A couple issues (I already mentioned them in slack but putting them here, too):

performance is pretty bad currently and probably won't scale. It would be good to investigate this further and make sure this is disabled for editor.deepforge.org.
Make sure the GitHub action is reverted so it isn't building on this branch

After ensuring it is disabled for editor.deepforge.org and the GitHub action is reverted, feel free to go ahead and merge it!

umesh-timalsina · 2021-01-21T19:49:41Z

In verbose mode, using the ConsoleWindow, we are getting the following error message:

DeepForge Language Client: Error: The task was cancelled.
    at https://dev.deepforge.org/extlib/src/visualizers/widgets/TextEditor/lib/vscode-ws-jsonrpc.min.js:1:8642
    at https://dev.deepforge.org/extlib/src/visualizers/widgets/TextEditor/lib/vscode-ws-jsonrpc.min.js:1:8936
    at https://dev.deepforge.org/extlib/src/visualizers/widgets/TextEditor/lib/vscode-ws-jsonrpc.min.js:1:9295
    at https://dev.deepforge.org/extlib/src/visualizers/widgets/TextEditor/lib/vscode-ws-jsonrpc.min.js:8:24724
    at p (https://dev.deepforge.org/extlib/src/visualizers/widgets/TextEditor/lib/vscode-ws-jsonrpc.min.js:8:24839)
    at a (https://dev.deepforge.org/extlib/src/visualizers/widgets/TextEditor/lib/vscode-ws-jsonrpc.min.js:8:24197)

This might be at the root of performance issue as new client gets instantiated as soon as this error is encountered. The minified files have no source maps, It might be good to investigate this further.

WIP #1908: Add working example with basic pyls

542f549

umesh-timalsina marked this pull request as draft September 17, 2020 21:52

umesh-timalsina added 2 commits September 21, 2020 17:18

Provide Dockerfile and build for a separate language servers service

7aeebe0

WIP- Update deployment of language servers and minor eslint fixes

da381fb

umesh-timalsina requested a review from brollb September 23, 2020 16:29

umesh-timalsina marked this pull request as ready for review September 23, 2020 16:29

brollb requested changes Sep 23, 2020

View reviewed changes

Use microsoft's python language server in favor of pyls(palantir)

2802a5d

umesh-timalsina added the do not merge label Sep 25, 2020

umesh-timalsina and others added 6 commits September 25, 2020 13:59

WIP- Fix deployment issue with language servers

4503ae4

WIP- Use components.json for languageserver configuration

29775a7

Merge remote-tracking branch 'origin/master' into 1908-pyls-integration

b777e43

WIP- Fix languageServers.json file; jq deployment; uninstall jsonrpc-…

08291c5

…ws-proxy; minor fixes to the TextEditorWidget

Merge remote-tracking branch 'origin/master' into 1908-pyls-integration

dd9fe28

WIP- Fix typo in config.extensions, rename token to tokens in languag…

37c1ddc

…eClient

umesh-timalsina removed the do not merge label Oct 2, 2020

umesh-timalsina requested a review from brollb October 2, 2020 15:14

umesh-timalsina added 4 commits October 2, 2020 12:14

WIP- Fix jsonrpc-ws-proxy call in Dockerfile.langservers entrypoint

03b41f5

WIP-Fix jsonrpc-ws-proxy in langserver-entrypoint.sh

c0fc77f

WIP_ Remove python-language-server from requirements

376fbc6

WIP- Fix typo in DeepforgeLanguageClient

dcd5ae5

brollb requested changes Oct 2, 2020

View reviewed changes

umesh-timalsina and others added 6 commits October 7, 2020 11:26

WIP- Use python3.7 anaconda in docker image

e26e7c3

WIP- Move languageServers.json to .deployment

f4facd8

WIP- Use inmemory models instead of file models

5ca1db3

Resolve merge conflicts and merge branch master

9e2525a

Merge remote-tracking branch 'origin/1908-pyls-integration' into 1908…

8bd0f3e

…-pyls-integration

Update README for deployment

ddbf23a

umesh-timalsina added 13 commits October 22, 2020 10:51

WIP- Use componentSettings for LanguageServers; Docker build for lang…

b1df3f6

…uageServers (test)

Merge remote-tracking branch 'origin/master' into 1908-pyls-integration

2bd224e

WIP- Test docker build for language servers

21ad734

WIP- Fix Dockerfile.langservers path

c8227d1

WIP- Add npm install step in github workflows

a339b9c

WIP- Don't update conda in Dockerfile

087323b

WIP- Use latest anaconda docker image

aa84bc2

Merge remote-tracking branch 'origin/master' into 1908-pyls-integration

a9e321a

WIP- Use anaconda3:2019.10 as image base

10bec66

Revert to pip installing packages

89b2e68

WIP- Fix Keras version in dockerfile

20bd21f

Merge remote-tracking branch 'origin/master' into 1908-pyls-integration

193698d

WIP- Minor fixes to the language server settings

d7b9a0d

umesh-timalsina added 3 commits January 20, 2021 13:03

Merge remote-tracking branch 'origin/master' into 1908-pyls-integration

aa60904

WIP- Deploy Branch for Testing

61bd70f

WIP- Publish KitchenSink image for testing

f75f652

umesh-timalsina added 4 commits January 20, 2021 13:30

WIP- Publish kitchen sink image for testing

c45823b

WIP- Add Push Tags

3d84cdb

WIP- Fix Tags for kitchensink image

d5bfa3f

WIP- Fix docker-compose for testing

ff110bc

umesh-timalsina added 2 commits January 20, 2021 15:11

WIP- Checkout branch in deployment script

36c6723

WIP- Fix branch checkout in workflow

81657c3

brollb approved these changes Jan 21, 2021

View reviewed changes

WIP- Fix entrypoint and setup debugging.

71979a3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate Python Language Server to Monaco Editor (Closes #1908, #571) #1910

Integrate Python Language Server to Monaco Editor (Closes #1908, #571) #1910

umesh-timalsina commented Sep 17, 2020 •

edited

Loading

brollb left a comment

brollb Sep 23, 2020

brollb Sep 23, 2020

brollb Sep 23, 2020

brollb Sep 23, 2020

umesh-timalsina Sep 23, 2020

brollb Sep 23, 2020

umesh-timalsina Sep 23, 2020

brollb Sep 23, 2020

umesh-timalsina Sep 23, 2020 •

edited

Loading

umesh-timalsina commented Sep 25, 2020

brollb left a comment

brollb Oct 2, 2020

brollb Oct 2, 2020

brollb Oct 2, 2020

brollb Oct 2, 2020

umesh-timalsina Oct 2, 2020

brollb commented Oct 2, 2020 •

edited

Loading

brollb commented Jan 8, 2021

umesh-timalsina commented Jan 8, 2021

umesh-timalsina commented Jan 20, 2021

brollb commented Jan 20, 2021

brollb commented Jan 21, 2021

umesh-timalsina commented Jan 21, 2021 •

edited

Loading


		const SERVERS_YML = path.join(__dirname, '..', 'language-servers.yml');

		function getAvailableLanguageServers () {

Integrate Python Language Server to Monaco Editor (Closes #1908, #571) #1910

Are you sure you want to change the base?

Integrate Python Language Server to Monaco Editor (Closes #1908, #571) #1910

Conversation

umesh-timalsina commented Sep 17, 2020 • edited Loading

brollb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

umesh-timalsina Sep 23, 2020 • edited Loading

Choose a reason for hiding this comment

umesh-timalsina commented Sep 25, 2020

brollb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brollb commented Oct 2, 2020 • edited Loading

brollb commented Jan 8, 2021

umesh-timalsina commented Jan 8, 2021

umesh-timalsina commented Jan 20, 2021

brollb commented Jan 20, 2021

brollb commented Jan 21, 2021

umesh-timalsina commented Jan 21, 2021 • edited Loading

umesh-timalsina commented Sep 17, 2020 •

edited

Loading

umesh-timalsina Sep 23, 2020 •

edited

Loading

brollb commented Oct 2, 2020 •

edited

Loading

umesh-timalsina commented Jan 21, 2021 •

edited

Loading