Implement back end -> front end comm messages #218

jmcphers · 2023-03-02T01:22:38Z

This change fleshes out a previously unimplemented part of the language runtime framework: specifically, it makes it possible for "comms" (client instances, such as the Environment pane) to send messages to the front end.

It also dramatically simplifies the code needed to implement a comm. Formerly, you had to provide a Rust struct that could be sent across threads (Send) and implemented a trait. After this change, the contract just gives you a Sender<Value> (where Value is any JSON data). Sending data to the channel causes it to be forwarded to the front end.

It works something like this:

graph LR
comm1 -- data --> receiver1 --> select -- metadata --> i[IOPub] -- zeromq --> Positron
comm2 -- data --> receiver2 --> select
comm3 -- data --> receiver3 --> select

That is: each comm gets its own unique Sender channel which has a unique Receiver channel. A dedicated thread runs a Select that waits for any of the comms to emit data. When a Receiver emits data, we look up the metadata associated with the comm and amend the message with the comm's ID. This gets further wrapped up with Jupyter headers and delivered to the front end via ZeroMQ.

Unit tests are included for the above that test sending data to and receiving data from a comm; the tests use a dummy comm implementation that just echoes any data sent to it. This implementation is also a useful reference for the simplest possible comm.

Finally, this change also adds retry logic to the R Language Pack's server connectivity. For now this is borrowed from the Python Language Pack, but we should have something more robust here (will file an issue).

kevinushey

Overall LGTM; only concern is around rustfmt noise.

kevinushey · 2023-03-04T00:41:34Z

extensions/positron-r/amalthea/crates/amalthea/src/comm/lsp_comm.rs


-    fn target_name(&self) -> String {
-        "lsp".to_string()
+    pub fn msg_rx(&self) -> Sender<CommChannelMsg> {


Function is called msg_rx but appears to return msg_tx; is that intentional?

I'm also confused how this would be used since we're just throwing away the created receiver.

Thanks, I've clarified the name and added a comment explaining this. In short, the LSP is sort of a dummy comm -- it has the same lifecycle (so we can start/stop/query it like a regular comm) but because it has its own TCP connection it doesn't send or receive messages over the Jupyter protocol.

kevinushey · 2023-03-04T00:45:17Z

extensions/positron-r/amalthea/crates/amalthea/tests/client.rs

@@ -274,10 +275,10 @@ fn test_kernel() {
            info!("Received execute reply: {:?}", reply);
            assert_eq!(reply.content.status, Status::Ok);
            assert_eq!(reply.content.execution_count, 2);
-        }
+        },


These trailing commas are kind of noisy; can we configure rustfmt to skip this sort of change?

(That is, I think either form is fine -- we should ask rustfmt to accept both has-trailing-comma and no-trailing-comma forms)

Alternatively, can we configure rustfmt to only run on regions of the file that we've explicitly modified?

I don't think it's possible for rustfmt to accept either one -- it is supposed to be opinionated. Nor is it possible to get it to run only on changed lines.

rust-lang/rustfmt#1324

kevinushey · 2023-03-04T00:47:36Z

extensions/jupyter-adapter/src/LanguageRuntimeAdapter.ts

@@ -248,32 +251,38 @@ export class LanguageRuntimeAdapter
 	 * Creates a new client instance.
 	 *
 	 * @param type The type of client to create
+	 * @param params The parameters for the client; the format of this object is


Are there any restrictions on this type (e.g. it must be a JavaScript object? it must be parsable as JSON?) I'm just wondering if there's something mildly more typed than any that we could use. But any is probably fine.

That's a good question. I checked the spec and it says that data is a dict, so I think that implies it has to be an object of some kind. I've updated the relevant types.

Merge pull request #218 from posit-dev/feature/force-quit Add force quit method -------------------- Commit message for posit-dev/positron-python@b06f70d: add force quit method Authored-by: Jonathan McPherson <jonathan@rstudio.com> Signed-off-by: Jonathan McPherson <jonathan@rstudio.com>

jmcphers added 26 commits February 22, 2023 16:12

add function for emitting a comm channel msg from kernel

66da9d4

FnOnce based emitter

bb40cfc

make message emitter static

b6bef59

use closure to send message over iopub socket

bce362e

Merge remote-tracking branch 'origin/main' into feature/comm-channel-msg

0c55078

send_request -> handle_request

2876028

use channels exclusively for delivery of msgs to front end

ea7ef59

use message types so we can emit close commands

affe88e

emit and handle closed/open messages

e8b1d6c

more robust error handling, comments

bee652d

graceful error handling for comm sockets

aaddaef

clarify methods & data in comm socket

4e58f7d

implement test comm

043f2ac

plumb initial client creation params through ext host

90a01dd

Merge remote-tracking branch 'origin/main' into feature/comm-channel-msg

6c5d9e8

promote lsp invocation to adapter level

3b041a6

implement client message and state emitters

d12bfb2

some plumbing for comm data

1fcae1d

deliver comm messages from kernel to adapter

eb6e419

separate message for comm closed from server side

3c9ef0a

finish plumbing for close messages

c956d7f

make comm open an async task

379dd34

port backoff-retry LSP connection to positron-r

4f74011

Merge remote-tracking branch 'origin/main' into feature/comm-channel-msg

bad940d

don't mix runtimes in adapter output

a6276a6

use kernel's log in more places

e35a090

petetronic requested a review from kevinushey March 3, 2023 21:45

kevinushey approved these changes Mar 4, 2023

View reviewed changes

jmcphers added 2 commits March 6, 2023 09:17

clarify lsp comm receiver

ffb7ecb

improve typing on comm data payload

2b376ba

jmcphers merged commit 15203fb into main Mar 6, 2023

wesm deleted the feature/comm-channel-msg branch March 15, 2024 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement back end -> front end comm messages #218

Implement back end -> front end comm messages #218

jmcphers commented Mar 2, 2023

kevinushey left a comment

kevinushey Mar 4, 2023

kevinushey Mar 4, 2023

jmcphers Mar 6, 2023

kevinushey Mar 4, 2023

kevinushey Mar 4, 2023

kevinushey Mar 4, 2023

jmcphers Mar 6, 2023

kevinushey Mar 4, 2023

jmcphers Mar 6, 2023

Implement back end -> front end comm messages #218

Implement back end -> front end comm messages #218

Conversation

jmcphers commented Mar 2, 2023

kevinushey 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

Choose a reason for hiding this comment