Module update

[filchristou]

Hello. Thanks for the awesome package :)
I 've been playing around to get RemoteREPL.jl to work well with Pluto.

The problem with Pluto is that everytime there is a change/evaluation in the notebook it creates a new Module.
So the client must explicitly change the module where the commands should be evaluated and that's tedious.

I tried to think of a non-intrusive way to solve this, but I couldn't wrap my head around it.
Instead here is an example of how RemoteREPL.jl could be modified.
Right now I have a global const Channel that the server can asynchronously modify with the putmodule! command.

Pluto side

You can see a Pluto notebook on gist
Basically it steps on the PlutoLinks.jl / PlutoHooks.jl to schedule an update of the module of RemoteREPL (with putmodule!) every second.
(I think this could be more clever but it's just a minimal working example)

Test

Run Pluto notebook and from the REPL do:

julia> using RemoteREPL, Sockets

julia> connect_repl(27755)
REPL mode remote_repl initialized. Press > to enter and backspace to exit.
"Prompt(\"julia@localhost:27755> \",...)"

julia@localhost:27755> @__MODULE__ # module changes automatically
Main.var"workspace#517"

julia@localhost:27755> @__MODULE__ # module changes automatically
Main.var"workspace#518"

julia> @remote(anewvar)
1

The RemoteREPL now will always be on the last update of the Pluto notebook.

This is just a draft.
I myself am not very happy with the restriction to have only one global Channel,
but before I move on I would like to know what do you think.
So basically two questions:

• Are you at all interested in any changes in this direction ?
• If yes, do you have any preferences for the implementation ? I will be happy to take them into consideration.

[c42f]

Hey, thanks! I'm definitely happy to make changes to support this use case :)

I think the "right way" to do this is to use the on_client_connect hook in serve_repl to grab a handle to the ServerSideSession each time a client connects. Then given the session you can just set session.in_module whenever Pluto creates a new module (does Pluto provide a hook so you know when this happens?) That way you don't need any globals.

The only tricky thing is that there's no on_client_disconnect hook in RemoteREPL so the pluto-side hooks won't know when to be detached from the session, even after the session is disconnected.

So I wonder what to do about that. We could add an on_client_disconnect hook, but that seems a bit awkward if the user wants to associate their own data structures with the session.

Maybe we should add a field to ServerSideSession ... like custom_session::Any or something, which can be set by on_client_connect and just call close() on it as part of close(::ServerSideSession).

That seems a bit wonky too, but I think it's approximately the right kind of thing - I think what we want here is some notion of "custom server side session".

[filchristou]

use the on_client_connect hook in serve_repl to grab a handle to the ServerSideSession each time a client connects

I guess to do this we need to wrap all the serve_repl inside a Channel similar to this dummy code

mutable struct MySession
    @atomic name::String
end

function setsessionname!(ses::MySession, newname)
    @atomic ses.name = newname
end

function dummy_serve(on_client_connect=nothing) 
    Channel{MySession}(1) do ch
        mysession = MySession("Main")
        if !isnothing(on_client_connect)
            on_client_connect(mysession)
            put!(ch, mysession)
        end
        i = 0
        while i < 5 
            sleep(2.0)
            @info "evaluating commands in $(mysession.name)"
            i += 1
        end
    end
end

and then the server (Pluto) will call it like:

julia> ses = take!(dummy_serve(x -> setsessionname!(x, "module#1")));
[ Info: evaluating commands in module#1
[ Info: evaluating commands in module#1
[ Info: evaluating commands in module#1

julia> setsessionname!(ses, "module#2");
[ Info: evaluating commands in module#2
[ Info: evaluating commands in module#2

But then what happens in on_client_connect is irrelative to returning an instance of ServerSideSession, and we could return the session eitherway.

Also what about mutexing ?
returning an instance of ServerSideSession that can be modified at any time might lead to corruption.
So probably the modification of session.in_module should be somewhat inside an @atomic or similar., which in my opinion should be provided from RemoteREPL.jl. Something like the setsessionname! above.

If we would return a Channel{Module}(1) then this could be handled this transparently, together with the closing:

The only tricky thing is that there's no on_client_disconnect hook in RemoteREPL so the pluto-side hooks won't know when to be detached from the session, even after the session is disconnected.

Maybe we should add a field to ServerSideSession ... like custom_session::Any or something, which can be set by on_client_connect and just call close() on it as part of close(::ServerSideSession).

Now that I think about it, the serve_repl caller can just check the session.socket to see if it's still open. That should make it clear whether it can continue updating the module or not. What else would you use this custom_session::Any for ?

[c42f]

I guess to do this we need to wrap all the serve_repl inside a Channel similar to this dummy code

This is one way to provide the sessions back to the caller. But I don't particularly love firing off that independent Task in Channel{MySession}(1) do ch. (Actually I wish that Channel constructor didn't exist in Base! It hides the concurrency in a way which I think is particularly confusing 😅)

I'd probably be inclined to have the user pass the channel instead. (Ugh, if only Julia had Go's select this would make all this stuff so much neater, and make clean cancellation much easier to do here.)

So probably the modification of session.in_module should be somewhat inside an @atomic or similar., which in my opinion should be provided from RemoteREPL.jl.

Yep this is a good point I think. "Unlikely" to cause a real problem, but data races are evil :-D

Actually a nicely appropriate way to fix this would be to communicate with ServerSideSession via the same message protocol that's used on the socket, but directly via request_chan in serve_repl_session rather than the socket. This is how it works from the client side and is async/thread safe... so why not the server side too?

I think this is the right way forward, it's just a little tricky because right now the code assumes that request_chan and response_chan are for a single remote client. So the response to the :in_module message will go to the remote client, not back to the pluto server. And that will break some assumptions in the protocol. To fix this, we'd have to be able to open multiple connections to the ServerSideSession. Which is doable and probably useful (multiple remote clients sharing a session - ipython can do this IIUC).

But if all that's too big of a rearrangement to contemplate, using an @atomic would probably be fine for now.

serve_repl caller can just check the session.socket to see if it's still open

This kind of polling generally leads to awkward code because it tends to look like

while true
    if !isopen(session.socket)
        break
    end
    do_something(session.socket)  # Is session.socket open here?
end

The awkwardness here is that session.socket can be closed asynchronously at any time so do_something() can fail. So I just try to avoid it where possible and use channels instead.

[c42f]

What else would you use this custom_session::Any for ?

Good question. Possibly it would be a place to stash any other session <--> pluto mappings you need?

But maybe that's not necessary and introduces extra coupling that we don't really need.

If the user can get hold of the ServerSideSession as it's generated in a nice async-safe way (via a channel I guess), and can communicate with those ServerSideSessions via the existing RemoteREPL message protocol, perhaps that covers everything?

In that case, they can wrap whatever logic they like around ServerSideSession, rather than having ServerSideSession wrap the logic inside itself. Seems like less coupling which is a good thing.

[filchristou]

@c42f Sorry for the delay. I had some time and I built something on the lines that you've suggested. Since it's rather different implementation I close this PR and I point you to #55 !

[fonsp]

Hey! It's been a while :) Could you remind me of the use case? Is this work for a RemoteREPL server in the Pluto notebook, and a client in a Julia REPL? So in your notebook you define x = 123, and then in the terminal you can remotely run x+x and get 246?

[filchristou]

hey hey. yes. Basically you should be able to access all the namespace of the newest notebook module.
the current PR is #55
We had to implement some further things but now it seems that some tests are failing.
I will look into that when I find some time.

The Pluto notebook looks like this https://gist.github.com/filchristou/80ab232ca1be67fb59de689913f41785
It's very simple: it basically sends the newest Module to the client every 1 second.

No changes are needed on the Pluto side.
But it would be convenient if you know of an event-based way of sending the newest Module upon a notebook change/evaluation, instead of polling it every a pre-defined time interval.