-
Notifications
You must be signed in to change notification settings - Fork 483
Implement Livebook Desktop with Tauri #3112
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
Conversation
|
|
||
| @impl true | ||
| def init(_) do | ||
| {:ok, pid} = ElixirKit.start() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to link those two processes anyway... also, what happens if ElixirKit is called multiple times?
Another option is for you to automatically start the ElixirKit process when ElixirKit starts and implement a pubsub using Elixir's Registry...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I'll revisit this soon. I even wanna rename ElixirKit.publish to ElixirKit.send exactly because I don't think we wanna build pubsub (one could fan out off ElixirKit listener process I suppose though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think the Rust -> Elixir side could be a pubsub (i.e. Rust publishes, Elixir subscribes), but Elixir -> Rust should be a send/receive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I think the idea behind starting Elixir tcp client on ElixirKit.start, as opposed to on elixirkit app supervision tree start, was if desktop app immediately starts sending messages there might not be a listener yet to receive those. One way would be to buffer those messages but then its arbitrary whether to flush the buffer on first listener vs any other listener which doesn't sound great.
So Elixir side could send a ready event and then Rust side would know to only start sending events then (which Livebook Desktop in fact does for another reason, sending the URL with token) but I thought that ElixirKit.start being a synchronization point out of the box was kinda nice.
For now I have introduced ElixirKit.start_link() and simplified internals so it's clearer there's just one process. I think we can implement Elixir side pubsub if and when someone asks for it. WDYT?
|
Gave the nightly build a try. |
|
@aayushmau5 thank you for checking! As you can see this is still being developed however that bug should be fixed now in nightly. |
josevalim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the remaining comments are addressed, LGTM!
Co-authored-by: Jonatan Kłosko <[email protected]>
|
I went ahead and merged this in, note we're still building the "current" Livebook Desktop and the "new" one. I'd like to do more testing before we retire the previous implementation for good. |
Already publishing as separate nightly builds: https://github.com/livebook-dev/livebook/releases/tag/nightly
TODO: