[APPS-41865] Add LSP plugin interface with pygls language server to formalize communication with vs code extension

[sfc-gh-klin]

Pre-review checklist

• I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
• I've added or updated automated unit tests to verify correctness of my new code.
• I've added or updated integration tests to verify correctness of my new code.
• I've confirmed that my changes are working by executing CLI's commands manually.
• I've confirmed that my changes are up-to-date with the target branch.
• I've described my changes in the release notes.
• I've described my changes in the section below.

Changes description

...

We propose adding code into SnowCLI that allows the VS Code extension to start a JSON-RPC Pygls language server and provides a new interface for “LSP plugins” so that the VS Code extension for Snowflake can make requests to this long-lived language server to invoke commands such as Native Apps commands instead of spawning one-off processes through the command line tool.

This lets us communicate between the two without parsing command line output. We can also formalize version compatibility rather than have the potential of SnowCLI changes breaking the VS Code extension for Snowflake.

Doc here
VS Code extension for Snowflake PR here: https://github.com/snowflakedb/snowflake-vscode-extension/pull/550

TODO:

• get some help writing integration tests to help prevent CLI changes breaking VS Code extension for Snowflake

[sfc-gh-turbaszek]

Should this be an enum? We are duplicating the naming here and in command decorators

[sfc-gh-turbaszek]

I'm really worried in how many places we have version strings.

[sfc-gh-klin]

Okay, I changed it to have a single version number for the lsp plugin, since that's what from pygls gets passed into ServerInfo to the client during init.

[sfc-gh-turbaszek]

This will need security consultation for risk of importing 3rd party modules.

[sfc-gh-klin]

What if I am only looking to import the internal ones? I don't see a need for the external packages.

[sfc-gh-turbaszek]

Is this guaranteed somewhere? Are we sure 3rd party plugins can't be loaded?

[sfc-gh-klin]

The way I am doing it now follows the path of calling load_only_builtin_command_plugins

def load_only_builtin_command_plugins() -> List[LoadedCommandPlugin]:
    loader = CommandPluginsLoader()
    loader.register_builtin_plugins()
    return loader.load_all_registered_plugins()

"get_builtin_plugin_name_to_plugin_spec" so it's only the stuff in builtin_plugins.py, which restricts it to the internal ones.

See load_lsp_plugins for where it calls the load_only_builtin_command_plugins.

[sfc-gh-turbaszek]

Left few comments, I'm mostly worried about how much code duplication we have at this moment.

[sfc-gh-klin]

@sfc-gh-turbaszek I've tried to use Pluggy to replace this discovery mechanism and I am stumped. I tried following a small toy example of it from their docs but I don't understand how the automatic plugin discovery is supposed to work, because it seems like it either needs some mapping the way the command_plugins_loader.py has get_builtin_plugin_name, or it uses the setuptools_entrypoints which I don't think I should need since there's no need for external plugins here.

[sfc-gh-klin]

It looks like SnowCLI uses the hook markers but then uses its own mechanism to identify the plugins. The LoadedCommandPlugin has a command_spec that gets used in "_load_command_spec" in command_plugins_loader to return the plugin.command_spec(), and the contents of that occur in plugin_spec.py for each plugin. The command_spec() for native apps for example is:

@plugin_hook_impl
def command_spec():
    return CommandSpec(
        parent_command_path=SNOWCLI_ROOT_COMMAND_PATH,
        command_type=CommandType.COMMAND_GROUP,
        typer_instance=commands.app.create_instance(),
    )

And it seems like the @plugin_hook_impl from the pluggy marker is not really used/needed by the builtin plugin process since the code doesn't use pluggy for discovery of those builtins. e.g., removing @plugin_hook_impl, everything still works.

So I guess I can follow the pattern there for command_spec but with another spec for the LSP stuff.

[sfc-gh-turbaszek]

If the LSP is only internal thing, and we don't expect plugins as packages why we are using pluggy? Do we really need automatic discovery? I think many things will become easier once we start implementing entities framework.

[sfc-gh-klin]

Using pluggy seemed more official and in line with the code in the repo than the way I was doing it before. Did you prefer the previous version that didn't use pluggy? In the end SnowCLI has its own mechanism to find the plugins; the plugin.command_spec and plugin.lsp_spec, so it isn't automatic discovery for the internal plugins. That's my current understanding.

[sfc-gh-cgorrie]

As a matter of style, it would make sense to direct clients to run a different server_command rather than using CLI terminology here.

[sfc-gh-klin]

Do you think it'd be good to have unique error names or error codes in these MessageResults that are given to the VS Code extension?

[sfc-gh-turbaszek]

@sfc-gh-klin what's the plan for end to end testing of LSP plugin? How we make sure that cli changes won't break VSC?

[sfc-gh-klin]

@sfc-gh-klin what's the plan for end to end testing of LSP plugin? How we make sure that cli changes won't break VSC?

I think having integration tests that start up the LSP server and then tests the interface would help ensure CLI changes don't break the extension. I'm not too well versed with writing integration tests in Python though so any pointers would be appreciated.

[sfc-gh-klin]

I tried to set up integration tests but my test is hanging and I'm trying to debug why. Do you guys have tips for how to get it to log some information when running? I'm running with python -m hatch to run my one integration test and naively trying to add repr() or logging.getLogger(name) and trying to log information doesn't work through that method.

[sfc-gh-klin]

🫡