Remove pluggy racing (#222)

* Completion: Remove pluggy racing and disable Rope by default

This allows the user to choose between Rope and Jedi for completions.

It also remove the pluggy racing since Jedi's cache isn't thread safe.

Closes #189

* Only use single completion hook

* Remove pluggy and disable rope completions by default

* Remove pluggy

pyls/_utils.py

@@ -89,33 +89,6 @@ def _merge_dicts_(a, b):
     return dict(_merge_dicts_(dict_a, dict_b))
 
 
-def race_hooks(hook_caller, pool, **kwargs):
-    """Given a pluggy hook spec, execute impls in parallel returning the first non-None result.
-
-    Note this does not support a lot of pluggy functionality, e.g. hook wrappers.
-    """
-    impls = hook_caller._nonwrappers + hook_caller._wrappers
-    log.debug("Racing hook impls for hook %s: %s", hook_caller, impls)
-
-    if not impls:
-        return None
-
-    def _apply(impl):
-        try:
-            return impl, impl.function(**kwargs)
-        except Exception:
-            log.exception("Failed to run hook %s", impl.plugin_name)
-            raise
-
-    # imap unordered gives us an iterator over the items in the order they finish.
-    # We have to be careful to set chunksize to 1 to ensure hooks each get their own thread.
-    # Unfortunately, there's no way to interrupt these threads, so we just have to leave them be.
-    for impl, result in pool.imap_unordered(_apply, impls, chunksize=1):
-        if result is not None:
-            log.debug("Hook from plugin %s returned: %s", impl.plugin_name, result)
-            return result
-
-
 def format_docstring(contents):
     """Python doc strings come in a number of formats, but LSP wants markdown.
 

pyls/plugins/jedi_completion.py

@@ -1,25 +1,21 @@
 # Copyright 2017 Palantir Technologies, Inc.
 import logging
-from pyls.lsp import CompletionItemKind
-from pyls import hookimpl, _utils
+from pyls import hookimpl, lsp, _utils
 
 log = logging.getLogger(__name__)
 
 
 @hookimpl
 def pyls_completions(document, position):
-    log.debug('Launching Jedi')
     definitions = document.jedi_script(position).completions()
-    definitions = [{
+    return [{
         'label': _label(d),
         'kind': _kind(d),
         'detail': _detail(d),
         'documentation': _utils.format_docstring(d.docstring()),
         'sortText': _sort_text(d),
         'insertText': d.name
-    } for d in definitions]
-    log.debug('Jedi finished')
-    return definitions
+    } for d in definitions] or None
 
 
 def _label(definition):
@@ -53,36 +49,36 @@ def _sort_text(definition):
 def _kind(d):
     """ Return the VSCode type """
     MAP = {
-        'none': CompletionItemKind.Value,
-        'type': CompletionItemKind.Class,
-        'tuple': CompletionItemKind.Class,
-        'dict': CompletionItemKind.Class,
-        'dictionary': CompletionItemKind.Class,
-        'function': CompletionItemKind.Function,
-        'lambda': CompletionItemKind.Function,
-        'generator': CompletionItemKind.Function,
-        'class': CompletionItemKind.Class,
-        'instance': CompletionItemKind.Reference,
-        'method': CompletionItemKind.Method,
-        'builtin': CompletionItemKind.Class,
-        'builtinfunction': CompletionItemKind.Function,
-        'module': CompletionItemKind.Module,
-        'file': CompletionItemKind.File,
-        'xrange': CompletionItemKind.Class,
-        'slice': CompletionItemKind.Class,
-        'traceback': CompletionItemKind.Class,
-        'frame': CompletionItemKind.Class,
-        'buffer': CompletionItemKind.Class,
-        'dictproxy': CompletionItemKind.Class,
-        'funcdef': CompletionItemKind.Function,
-        'property': CompletionItemKind.Property,
-        'import': CompletionItemKind.Module,
-        'keyword': CompletionItemKind.Keyword,
-        'constant': CompletionItemKind.Variable,
-        'variable': CompletionItemKind.Variable,
-        'value': CompletionItemKind.Value,
-        'param': CompletionItemKind.Variable,
-        'statement': CompletionItemKind.Keyword,
+        'none': lsp.CompletionItemKind.Value,
+        'type': lsp.CompletionItemKind.Class,
+        'tuple': lsp.CompletionItemKind.Class,
+        'dict': lsp.CompletionItemKind.Class,
+        'dictionary': lsp.CompletionItemKind.Class,
+        'function': lsp.CompletionItemKind.Function,
+        'lambda': lsp.CompletionItemKind.Function,
+        'generator': lsp.CompletionItemKind.Function,
+        'class': lsp.CompletionItemKind.Class,
+        'instance': lsp.CompletionItemKind.Reference,
+        'method': lsp.CompletionItemKind.Method,
+        'builtin': lsp.CompletionItemKind.Class,
+        'builtinfunction': lsp.CompletionItemKind.Function,
+        'module': lsp.CompletionItemKind.Module,
+        'file': lsp.CompletionItemKind.File,
+        'xrange': lsp.CompletionItemKind.Class,
+        'slice': lsp.CompletionItemKind.Class,
+        'traceback': lsp.CompletionItemKind.Class,
+        'frame': lsp.CompletionItemKind.Class,
+        'buffer': lsp.CompletionItemKind.Class,
+        'dictproxy': lsp.CompletionItemKind.Class,
+        'funcdef': lsp.CompletionItemKind.Function,
+        'property': lsp.CompletionItemKind.Property,
+        'import': lsp.CompletionItemKind.Module,
+        'keyword': lsp.CompletionItemKind.Keyword,
+        'constant': lsp.CompletionItemKind.Variable,
+        'variable': lsp.CompletionItemKind.Variable,
+        'value': lsp.CompletionItemKind.Value,
+        'param': lsp.CompletionItemKind.Variable,
+        'statement': lsp.CompletionItemKind.Keyword,
     }
 
     return MAP.get(d.type)

pyls/plugins/rope_completion.py

@@ -9,9 +9,13 @@
 
 
 @hookimpl
-def pyls_completions(document, position):
-    log.debug('Launching Rope')
+def pyls_settings():
+    # Default rope_completion to disabled
+    return {'plugins': {'rope_completion': {'enabled': False}}}
+
 
+@hookimpl
+def pyls_completions(document, position):
     # Rope is a bit rubbish at completing module imports, so we'll return None
     word = document.word_at_position({
         # The -1 should really be trying to look at the previous word, but that might be quite expensive
@@ -40,7 +44,7 @@ def pyls_completions(document, position):
             'documentation': doc or "",
             'sortText': _sort_text(d)})
     definitions = new_definitions
-    log.debug('Rope finished')
+
     return definitions or None
 
 

pyls/python_ls.py

@@ -1,14 +1,12 @@
 # Copyright 2017 Palantir Technologies, Inc.
 import logging
-from multiprocessing import dummy as multiprocessing
 from . import lsp, _utils
 from .config import config
 from .language_server import LanguageServer
 from .workspace import Workspace
 
 log = logging.getLogger(__name__)
 
-PLUGGY_RACE_POOL_SIZE = 5
 LINT_DEBOUNCE_S = 0.5  # 500 ms
 
 
@@ -21,8 +19,6 @@ class PythonLanguageServer(LanguageServer):
     # Set of method dispatchers to query
     _dispatchers = []
 
-    _pool = multiprocessing.Pool(PLUGGY_RACE_POOL_SIZE)
-
     def __getitem__(self, item):
         """Override the method dispatcher to farm out any unknown messages to our plugins."""
         try:
@@ -41,7 +37,8 @@ def _hook_caller(self, hook_name):
 
     def _hook(self, hook_name, doc_uri=None, **kwargs):
         doc = self.workspace.get_document(doc_uri) if doc_uri else None
-        return self._hook_caller(hook_name)(config=self.config, workspace=self.workspace, document=doc, **kwargs)
+        hook = self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins)
+        return hook(config=self.config, workspace=self.workspace, document=doc, **kwargs)
 
     def capabilities(self):
         server_capabilities = {
@@ -85,14 +82,10 @@ def code_lens(self, doc_uri):
         return flatten(self._hook('pyls_code_lens', doc_uri))
 
     def completions(self, doc_uri, position):
-        completions = _utils.race_hooks(
-            self._hook_caller('pyls_completions'), self._pool,
-            document=self.workspace.get_document(doc_uri) if doc_uri else None,
-            position=position
-        )
+        completions = self._hook('pyls_completions', doc_uri, position=position)
         return {
             'isIncomplete': False,
-            'items': completions or []
+            'items': flatten(completions)
         }
 
     def definitions(self, doc_uri, position):

setup.py

@@ -61,7 +61,6 @@
             'pyls = pyls.__main__:main',
         ],
         'pyls': [
-            'rope_completion = pyls.plugins.rope_completion',
             'jedi_completion = pyls.plugins.jedi_completion',
             'jedi_definition = pyls.plugins.definition',
             'jedi_hover = pyls.plugins.hover',
@@ -72,6 +71,7 @@
             'pycodestyle = pyls.plugins.pycodestyle_lint',
             'pydocstyle = pyls.plugins.pydocstyle_lint',
             'pyflakes = pyls.plugins.pyflakes_lint',
+            'rope_completion = pyls.plugins.rope_completion',
             'rope_rename = pyls.plugins.rope_rename',
             'yapf = pyls.plugins.format',
         ]