feat: Add "deno jupyter" subcommand #20337
Conversation
|
|
||
| if let Some(specs) = json_output.get("kernelspecs") { | ||
| if let Some(specs_obj) = specs.as_object() { | ||
| if specs_obj.contains_key("deno") { |
There was a problem hiding this comment.
Should we validate that this object matches what would be installed?
There was a problem hiding this comment.
I don't think it's needed, especially that it might change between jupyter versions.
| // handling stdout and stderr streams to receive and flush the content. | ||
| // Otherwise, executing multiple cells one-by-one might lead to output | ||
| // from various cells be grouped together in another cell result. | ||
| tokio::time::sleep(std::time::Duration::from_millis(5)).await; |
There was a problem hiding this comment.
This seems a little fragile but maybe we can add a TODO to replace it with a mutex or other lock?
There was a problem hiding this comment.
There's nothing to mutex/lock on - it's the fact that channels need a moment to flush all data that might have been printed/console logged. I haven't found a better solution to this problem.
There was a problem hiding this comment.
Fair enough. Looks like the protocol for the kernels doesn't seem to give us any way to actually route these properly. We can always adjust in the future if the heuristic doesn't work.
There was a problem hiding this comment.
FWIW it seems like ipython's kernel does the same thing
There was a problem hiding this comment.
Protocol wise this can be handled properly by sending the right parent_header.msg_id. However that would mean tracking the parent message ID for each execution which might not be in the right cell during async code.
On the Jupyter front ends we determine all the outputs for a cell by grouping on the parent--header.msg_id
For the sake of this PR or an upcoming release though... not worth it
|
Great to see this being merged @bartlomieju! Any clue on how to start using it on vscode? |
|
@ceifa if you build Deno from source on latest |
|
Great work, @bartlomieju! |
|
Kind of just adding noise here, and it maybe sounds overdramatic, but I just want to say: Thank you so much for experimenting with stuff like this! Making things compatible and fast is obviously great & important, but the fact that Deno is really willing to push the JS ecosystem into new areas (granular security being the headliner, of course) is what actually makes me excited about the Deno project - it's a large par of the reason why I recommend it to people, submit bug report, put up with early issues, etc. Deno has made the server-side JS world exciting for me. (Also, while I'm on this topic, if you're looking for new ground to break in the "all-in-one, just-works" department, a built-in pm2-like system would be awesome. Memory limits, auto restart on file changes, start on server boot, uptime/memory/etc tracking, clustering, etc. I'm currently using pm2, but keeping an eye on pup) |
|
Also noise: The minute this lands I'll start using it (I do a lot of data exploration in typescript). Two years in the making and it's coming, exciting. |
|
@josephrocca @guy-borderless you can now try it out :) |
|
working perfectly so far, I'll test more and report. small problem but launching from the cli does not work on my machine,
the VSCode jupyter extension doesn't seem to detect the deno kernel, not when pressing "Select Kernel" and giving the path, not when pressing "python" (or in the marketplace with tag @tag:notebookKernelJupyterNotebook maybe add it there, or install it by default with deno if you detect jupyter) |
|
@guy-borderless can you open an new issue about it? |
|
Please feel free to ping me on the issue you make @guy-borderless. I've got some diagnostic steps I can help you with in a new thread. |
thanks I'll open an issue soon |
This is #13122 rebased against
main.Closes #13016