Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor SphinxManager and SubprocessSphinxClient #744

Merged
merged 6 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/esbonio/esbonio/server/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from .feature import LanguageFeature
from .server import EsbonioLanguageServer
from .server import EsbonioWorkspace
from .setup import create_language_server

__all__ = (
"LOG_NAMESPACE",
Expand All @@ -21,4 +22,5 @@
"LanguageFeature",
"MemoryHandler",
"Uri",
"create_language_server",
)
76 changes: 48 additions & 28 deletions lib/esbonio/esbonio/server/_configuration.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from __future__ import annotations

import asyncio
import inspect
import json
import pathlib
import traceback
import typing
from functools import partial
from typing import Generic
from typing import TypeVar

Expand Down Expand Up @@ -111,6 +114,9 @@ def __init__(self, server: EsbonioLanguageServer):
self._subscriptions: Dict[Subscription, Any] = {}
"""Subscriptions and their last known value"""

self._tasks: Set[asyncio.Task] = set()
"""Holds tasks that are currently executing an async config handler."""

@property
def initialization_options(self):
return self._initialization_options
Expand Down Expand Up @@ -141,7 +147,7 @@ def supports_workspace_config(self):
self.server.client_capabilities, "workspace.configuration", False
)

async def subscribe(
def subscribe(
self,
section: str,
spec: Type[T],
Expand Down Expand Up @@ -174,27 +180,12 @@ async def subscribe(
self.logger.debug("Ignoring duplicate subscription: %s", subscription)
return

# Wait until the server is ready before fetching the initial configuration
await self.server.ready

result = self.get(section, spec, scope)
self._subscriptions[subscription] = result

change_event = ConfigChangeEvent(
scope=max([file_scope, workspace_scope], key=len), value=result
)
self.logger.info("%s", change_event)
self._subscriptions[subscription] = None

try:
ret = callback(change_event)
if inspect.isawaitable(ret):
await ret
except Exception:
self.logger.error(
"Error in configuration callback: %s", callback, exc_info=True
)
# Once the server is ready, update all the subscriptions
self.server.ready.add_done_callback(self._notify_subscriptions)

async def _notify_subscriptions(self):
def _notify_subscriptions(self, *args):
"""Notify subscriptions about configuration changes, if necessary."""

for subscription, previous_value in self._subscriptions.items():
Expand All @@ -211,6 +202,7 @@ async def _notify_subscriptions(self):
if previous_value == value:
continue

self._subscriptions[subscription] = value
change_event = ConfigChangeEvent(
scope=max(
[subscription.file_scope, subscription.workspace_scope], key=len
Expand All @@ -222,17 +214,28 @@ async def _notify_subscriptions(self):

try:
ret = subscription.callback(change_event)
if inspect.isawaitable(ret):
await ret
if inspect.iscoroutine(ret):
task = asyncio.create_task(ret)
task.add_done_callback(partial(self._finish_task, subscription))

self._subscriptions[subscription] = value
except Exception:
self.logger.error(
"Error in configuration callback: %s",
subscription.callback,
exc_info=True,
)

def _finish_task(self, subscription: Subscription, task: asyncio.Task[None]):
"""Cleanup a finished task."""
self._tasks.discard(task)

if (exc := task.exception()) is not None:
self.logger.error(
"Error in async configuration handler '%s'\n%s",
subscription.callback,
traceback.format_exception(type(exc), exc, exc.__traceback__),
)

def get(self, section: str, spec: Type[T], scope: Optional[Uri] = None) -> T:
"""Get the requested configuration section.

Expand All @@ -257,6 +260,25 @@ def get(self, section: str, spec: Type[T], scope: Optional[Uri] = None) -> T:

return self._get_config(section, spec, workspace_scope, file_scope)

def scope_for(self, uri: Uri) -> str:
"""Return the configuration scope that corresponds to the given uri.

Parameters
----------
uri
The uri to return the scope for

Returns
-------
str
The scope corresponding with the given uri
"""

file_scope = self._uri_to_file_scope(uri)
workspace_scope = self._uri_to_workspace_scope(uri)

return max([file_scope, workspace_scope], key=len)

def _get_config(
self, section: str, spec: Type[T], workspace_scope: str, file_scope: str
) -> T:
Expand Down Expand Up @@ -327,9 +349,7 @@ def _discover_config_files(self) -> List[pathlib.Path]:

return paths

async def update_file_configuration(
self, paths: Optional[List[pathlib.Path]] = None
):
def update_file_configuration(self, paths: Optional[List[pathlib.Path]] = None):
"""Update the internal cache of configuration coming from files.

Parameters
Expand All @@ -356,7 +376,7 @@ async def update_file_configuration(
"Unable to read configuration file: '%s'", exc_info=True
)

await self._notify_subscriptions()
self._notify_subscriptions()

async def update_workspace_configuration(self):
"""Update the internal cache of the client's workspace configuration."""
Expand Down Expand Up @@ -397,7 +417,7 @@ async def update_workspace_configuration(self):

self._workspace_config[scope or ""] = result

await self._notify_subscriptions()
self._notify_subscriptions()


def _uri_to_scope(known_scopes: List[str], uri: Optional[Uri]) -> str:
Expand Down
4 changes: 2 additions & 2 deletions lib/esbonio/esbonio/server/features/directives/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ def add_provider(self, provider: DirectiveProvider):

completion_triggers = [RST_DIRECTIVE, MYST_DIRECTIVE]

async def initialized(self, params: types.InitializedParams):
def initialized(self, params: types.InitializedParams):
"""Called once the initial handshake between client and server has finished."""
await self.configuration.subscribe(
self.configuration.subscribe(
"esbonio.server.completion",
server.CompletionConfig,
self.update_configuration,
Expand Down
4 changes: 2 additions & 2 deletions lib/esbonio/esbonio/server/features/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ class LogManager(server.LanguageFeature):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

async def initialized(self, params: types.InitializedParams):
def initialized(self, params: types.InitializedParams):
"""Setup logging."""
await self.server.configuration.subscribe(
self.server.configuration.subscribe(
"esbonio.server", ServerLogConfig, self.setup_logging
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,17 @@

from esbonio.server import EsbonioLanguageServer

from .client import ClientState
from .client import SphinxClient
from .client_mock import MockSphinxClient
from .client_mock import mock_sphinx_client_factory
from .client_subprocess import make_subprocess_sphinx_client
from .config import SphinxConfig
from .manager import SphinxManager

__all__ = [
"ClientState",
"SphinxClient",
"SphinxConfig",
"SphinxManager",
"MockSphinxClient",
"mock_sphinx_client_factory",
]


Expand Down
53 changes: 39 additions & 14 deletions lib/esbonio/esbonio/server/features/sphinx_manager/client.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from __future__ import annotations

import enum
import typing
from typing import Protocol

if typing.TYPE_CHECKING:
from typing import Any
from typing import Dict
from typing import Generator
from typing import List
from typing import Optional
from typing import Tuple
Expand All @@ -15,47 +17,70 @@
from esbonio.server import Uri
from esbonio.sphinx_agent import types

from .config import SphinxConfig

class ClientState(enum.Enum):
"""The set of possible states the client may be in."""

Starting = enum.auto()
"""The client is starting."""

Running = enum.auto()
"""The client is running normally."""

Building = enum.auto()
"""The client is currently building."""

Errored = enum.auto()
"""The client has enountered some unrecoverable error and should not be used."""

Exited = enum.auto()
"""The client is no longer running."""


class SphinxClient(Protocol):
"""Describes the API language features can use to inspect/manipulate a Sphinx
application instance."""

state: Optional[ClientState]
sphinx_info: Optional[types.SphinxInfo]

@property
def id(self) -> Optional[str]:
def id(self) -> str:
"""The id of the Sphinx instance."""
...

@property
def db(self) -> Optional[aiosqlite.Connection]:
"""Connection to the associated database."""

@property
def builder(self) -> Optional[str]:
def builder(self) -> str:
"""The name of the Sphinx builder."""
...

@property
def building(self) -> bool:
"""Indicates if the Sphinx instance is building."""

@property
def build_uri(self) -> Optional[Uri]:
def build_uri(self) -> Uri:
"""The URI to the Sphinx application's build dir."""
...

@property
def conf_uri(self) -> Optional[Uri]:
def conf_uri(self) -> Uri:
"""The URI to the Sphinx application's conf dir."""
...

@property
def src_uri(self) -> Optional[Uri]:
def src_uri(self) -> Uri:
"""The URI to the Sphinx application's src dir."""
...

async def start(self, config: SphinxConfig):
"""Start the client."""
def __await__(self) -> Generator[Any, None, SphinxClient]:
"""A SphinxClient should be awaitable"""
...

async def create_application(self, config: SphinxConfig) -> types.SphinxInfo:
"""Create a new Sphinx application instance."""
def add_listener(self, event: str, handler): ...

async def start(self) -> SphinxClient:
"""Start the client."""
...

async def build(
Expand Down
Loading
Loading