add kernel-sidecar #16

kafonek · 2024-01-14T01:09:42Z

Leaving this in draft status but putting it up so people can try it out and tell me which direction they want to go for an initial prototype of integrating kernel-sidecar.

replaces the Notebook and Cell models with those from kernel-sidecar
starts an ipykernel process as a subprocess on cell execution (doesn't clean up if you x out of the tauri app)
prints out the ZMQ responses during cell execution, doesn't update the UI right now

kafonek · 2024-01-14T01:09:55Z

Current dependencies on/for this PR:

main
- PR add kernel-sidecar #16 👈

This stack of pull requests is managed by Graphite.

rgbkrk · 2024-01-14T01:45:41Z

It looks like we'll have to change our Cargo toml with custom instructions for windows.

error[E0432]: unresolved imports `tokio::net::UnixListener`, `tokio::net::UnixStream`
  --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\zeromq-0.3.4\src\transport\ipc.rs:2:18
   |
2  | use tokio::net::{UnixListener, UnixStream};
   |                  ^^^^^^^^^^^^  ^^^^^^^^^^ no `UnixStream` in `net`
   |                  |
   |                  no `UnixListener` in `net`
   |                  help: a similar name exists in the module: `TcpListener`
   |
note: found an item that was configured out
  --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.35.1\src\net\mod.rs:50:29
   |
50 |     pub use unix::listener::UnixListener;
   |                             ^^^^^^^^^^^^
note: found an item that was configured out
  --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.35.1\src\net\mod.rs:51:27
   |
51 |     pub use unix::stream::UnixStream;
   |                           ^^^^^^^^^^

Deno probably has the right bits.

rgbkrk · 2024-01-14T01:48:35Z

src-tauri/src/main.rs

@@ -83,65 +108,18 @@ impl AppState {

        let mut notebooks = self.notebooks.lock().await;
        if let Some(notebook) = notebooks.get_mut(window_id) {
-            notebook.update_cell(cell_id, new_content);
+            if let Some(cell) = notebook.get_mut_cell(cell_id) {
+                cell.set_source(new_content);


rgbkrk · 2024-01-14T01:49:32Z

src-tauri/src/main.rs

-    state: State<'_, AppState>,
-    window: Window,
-) -> Result<Option<String>, String> {
+async fn create_cell(state: State<'_, AppState>, window: Window) -> Result<Option<String>, String> {


Took me a bit to realize this was just a formatting change. I wonder if I'm on a different rust version locally or something.

Good point, what do you want to do with formatting? I ran cargo +nightly fmt / cargo clippy --fix with a rustfmt.toml that has the line imports_granularity = "Module" (not relevant to the line you have here, just on import formatting).

I just let VS Code do its thing with clippy. I wouldn't be surprised if I have some other setting interfering. No biggie for now, I can just follow your process on it when I commit.

rgbkrk · 2024-01-14T01:53:11Z

src-tauri/src/main.rs

+
+    async fn start_kernel(&self, window_id: &str) -> (JupyterKernel, Client) {
+        info!("Starting kernel for window with ID: {}", window_id);
+        let silent = true; // true = send ipykernel subprocess stdout to /dev/null


I'd love to be able to collect the stdout/stderr from the subprocess, mostly for debugging. It's very useful for setups involving background work like Spark.

Now that I see how the process is spawned, we can leave this to future work.

rgbkrk · 2024-01-14T01:53:53Z

src-tauri/src/main.rs

+    async fn start_kernel(&self, window_id: &str) -> (JupyterKernel, Client) {
+        info!("Starting kernel for window with ID: {}", window_id);
+        let silent = true; // true = send ipykernel subprocess stdout to /dev/null
+        let kernel = JupyterKernel::ipython(silent);


Based on this, is the process launching synchronous?

Yeah, using std::process::Command there. Looks like there's a crate for async process, haven't tried it out https://docs.rs/async-process/latest/async_process/struct.Command.html

You think spawning the process should be async? I can create an issue in kernel-sidecar and research it

Tauri has a built in for launching an asynchronous process, which I'm guessing wraps around tokio. We can touch this later since it's not a blocker at the moment.

rgbkrk · 2024-01-14T01:57:12Z

Future direction I'm curious about for us is if/how we're going to do events. I see how events are done both in the Rust backend and JS frontend in the Tauri docs: https://tauri.app/v1/guides/features/events/. What I'm not sure about is how we're going to receive updates from kernel sidecar and send those events on to the client.

rgbkrk · 2024-01-14T23:15:20Z

~~I'll start hacking on a frontend receiver for cell and output updates in a new branch.~~

Update: did it here

…tion

rgbkrk · 2024-01-15T15:08:14Z

Looks like zeromq/zmq.rs#185 did the trick. Thanks @mmastrac for the quick ship. 🚀

src-tauri/Cargo.toml

kafonek force-pushed the mrk/kernel-sidecar-initial branch from 1816a75 to 3338523 Compare January 14, 2024 01:10

kafonek marked this pull request as draft January 14, 2024 01:11

rgbkrk reviewed Jan 14, 2024

View reviewed changes

kafonek added 3 commits January 14, 2024 16:06

add kernel-sidecar

7d90559

add basic kernel sidecar integration, definitely needs refactoring

4efb993

hacking with Kyle, emit outputs to frontend in really naive serializa…

8b57149

…tion

rgbkrk force-pushed the mrk/kernel-sidecar-initial branch 2 times, most recently from f7749a7 to 084fdf1 Compare January 15, 2024 01:00

set up an event listener for outputs within useCell

b3f50f0

rgbkrk force-pushed the mrk/kernel-sidecar-initial branch from 084fdf1 to b3f50f0 Compare January 15, 2024 01:49

rgbkrk added 3 commits January 14, 2024 18:09

try to pin down versions for builds

8c4157c

attempt to pin down *nix vs windows for transport

6b33d3e

turn off default features of zeromq

1c2df19

rgbkrk mentioned this pull request Jan 15, 2024

only build ipc transport on *nix systems zeromq/zmq.rs#185

Merged

bump zeromq

4843185

rgbkrk marked this pull request as ready for review January 15, 2024 15:07

rgbkrk reviewed Jan 15, 2024

View reviewed changes

src-tauri/Cargo.toml Show resolved Hide resolved

bring tokio back to fully featured

0a45425

rgbkrk approved these changes Jan 15, 2024

View reviewed changes

rgbkrk merged commit da4ac0b into main Jan 15, 2024
3 checks passed

rgbkrk deleted the mrk/kernel-sidecar-initial branch January 15, 2024 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add kernel-sidecar #16

add kernel-sidecar #16

kafonek commented Jan 14, 2024 •

edited

Loading

kafonek commented Jan 14, 2024

rgbkrk commented Jan 14, 2024

rgbkrk Jan 14, 2024

rgbkrk Jan 14, 2024

kafonek Jan 14, 2024

rgbkrk Jan 14, 2024

rgbkrk Jan 14, 2024

rgbkrk Jan 14, 2024

rgbkrk Jan 14, 2024

kafonek Jan 14, 2024

rgbkrk Jan 14, 2024

rgbkrk commented Jan 14, 2024

rgbkrk commented Jan 14, 2024 •

edited

Loading

rgbkrk commented Jan 15, 2024

add kernel-sidecar #16

add kernel-sidecar #16

Conversation

kafonek commented Jan 14, 2024 • edited Loading

kafonek commented Jan 14, 2024

rgbkrk commented Jan 14, 2024

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

rgbkrk commented Jan 14, 2024

rgbkrk commented Jan 14, 2024 • edited Loading

rgbkrk commented Jan 15, 2024

kafonek commented Jan 14, 2024 •

edited

Loading

rgbkrk commented Jan 14, 2024 •

edited

Loading