Skip to content
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.

Sign up for GitHub

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

fix(jupyter): more robust Deno.jupyter namespace #20710

Merged
merged 7 commits into from
Sep 28, 2023
Merged

fix(jupyter): more robust Deno.jupyter namespace #20710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1491421
fix(cli): panic with __runtime_js_sources
lucacasonato Sep 27, 2023
db919b7
Merge branch 'main' into fix_jupyter_panic
bartlomieju Sep 27, 2023
9821a0d
fix reassigning the namespace
bartlomieju Sep 27, 2023
f89c50f
ensure fast ops, update test
bartlomieju Sep 27, 2023
5f29a3d
don't send reply if mime bundle is empty
bartlomieju Sep 27, 2023
dfe9b29
Merge branch 'main' into fix_jupyter_test
bartlomieju Sep 27, 2023
1c3485f
reset CI
bartlomieju Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions cli/js/40_jupyter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
const core = globalThis.Deno.core;
const internals = globalThis.__bootstrap.internals;

import { denoNsUnstable } from "ext:runtime/90_deno_ns.js";

function enableJupyter() {
denoNsUnstable.jupyter = {
const {
op_jupyter_broadcast,
} = core.ensureFastOps();

globalThis.Deno.jupyter = {
async broadcast(msgType, content) {
await core.opAsync("op_jupyter_broadcast", msgType, content);
await op_jupyter_broadcast(msgType, content);
},
};
}
Expand Down
107 changes: 57 additions & 50 deletions cli/tests/testdata/jupyter/integration_test.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@
},
{
"cell_type": "code",
"execution_count": 21,
"execution_count": 1,
"id": "a5d38758",
"metadata": {
"hidden": true
},
"outputs": [
{
"data": {},
"execution_count": 21,
"execution_count": 1,
"metadata": {},
"output_type": "execute_result"
},
Expand Down Expand Up @@ -81,15 +81,15 @@
},
{
"cell_type": "code",
"execution_count": 22,
"execution_count": 2,
"id": "f7fa885a",
"metadata": {
"hidden": true
},
"outputs": [
{
"data": {},
"execution_count": 22,
"execution_count": 2,
"metadata": {},
"output_type": "execute_result"
},
Expand Down Expand Up @@ -120,7 +120,7 @@
},
{
"cell_type": "code",
"execution_count": 23,
"execution_count": 3,
"id": "08a17340",
"metadata": {
"hidden": true
Expand All @@ -132,7 +132,7 @@
"{ color: \u001b[32m\"red\"\u001b[39m, area: \u001b[33m10000\u001b[39m }"
]
},
"execution_count": 23,
"execution_count": 3,
"metadata": {},
"output_type": "execute_result"
}
Expand Down Expand Up @@ -177,15 +177,15 @@
},
{
"cell_type": "code",
"execution_count": 24,
"execution_count": 4,
"id": "bbf2c09b",
"metadata": {
"hidden": true
},
"outputs": [
{
"data": {},
"execution_count": 24,
"execution_count": 4,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -207,7 +207,7 @@
},
{
"cell_type": "code",
"execution_count": 25,
"execution_count": 5,
"id": "d9801d80",
"metadata": {
"hidden": true
Expand All @@ -219,7 +219,7 @@
"\u001b[1mnull\u001b[22m"
]
},
"execution_count": 25,
"execution_count": 5,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -241,7 +241,7 @@
},
{
"cell_type": "code",
"execution_count": 26,
"execution_count": 6,
"id": "cfaac330",
"metadata": {
"hidden": true
Expand All @@ -253,7 +253,7 @@
"\u001b[33mtrue\u001b[39m"
]
},
"execution_count": 26,
"execution_count": 6,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -275,7 +275,7 @@
},
{
"cell_type": "code",
"execution_count": 27,
"execution_count": 7,
"id": "ec3be2da",
"metadata": {
"hidden": true
Expand All @@ -287,7 +287,7 @@
"\u001b[33m42\u001b[39m"
]
},
"execution_count": 27,
"execution_count": 7,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -309,7 +309,7 @@
},
{
"cell_type": "code",
"execution_count": 28,
"execution_count": 8,
"id": "997cf2d7",
"metadata": {
"hidden": true
Expand All @@ -321,7 +321,7 @@
"\u001b[32m\"this is a test of the emergency broadcast system\"\u001b[39m"
]
},
"execution_count": 28,
"execution_count": 8,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -343,7 +343,7 @@
},
{
"cell_type": "code",
"execution_count": 29,
"execution_count": 9,
"id": "44b63807",
"metadata": {
"hidden": true
Expand All @@ -355,7 +355,7 @@
"\u001b[33m31337n\u001b[39m"
]
},
"execution_count": 29,
"execution_count": 9,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -377,7 +377,7 @@
},
{
"cell_type": "code",
"execution_count": 30,
"execution_count": 10,
"id": "e10c0d31",
"metadata": {
"hidden": true
Expand All @@ -389,7 +389,7 @@
"\u001b[32mSymbol(foo)\u001b[39m"
]
},
"execution_count": 30,
"execution_count": 10,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -411,7 +411,7 @@
},
{
"cell_type": "code",
"execution_count": 31,
"execution_count": 11,
"id": "81c99233",
"metadata": {
"hidden": true
Expand All @@ -423,7 +423,7 @@
"{ foo: \u001b[32m\"bar\"\u001b[39m }"
]
},
"execution_count": 31,
"execution_count": 11,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -445,7 +445,7 @@
},
{
"cell_type": "code",
"execution_count": 32,
"execution_count": 12,
"id": "43c1581b",
"metadata": {
"hidden": true
Expand All @@ -457,7 +457,7 @@
"Promise { \u001b[32m\"it worked!\"\u001b[39m }"
]
},
"execution_count": 32,
"execution_count": 12,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -468,7 +468,7 @@
},
{
"cell_type": "code",
"execution_count": 33,
"execution_count": 13,
"id": "9a34b725",
"metadata": {
"hidden": true
Expand All @@ -483,7 +483,7 @@
"}"
]
},
"execution_count": 33,
"execution_count": 13,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -494,7 +494,7 @@
},
{
"cell_type": "code",
"execution_count": 34,
"execution_count": 14,
"id": "b5c7b819",
"metadata": {
"scrolled": true
Expand All @@ -520,7 +520,7 @@
},
{
"cell_type": "code",
"execution_count": 35,
"execution_count": 15,
"id": "14844fc9-536e-4121-a9bd-fc2d3f7b6395",
"metadata": {},
"outputs": [
Expand All @@ -541,21 +541,23 @@
},
{
"cell_type": "code",
"execution_count": 36,
"execution_count": 16,
"id": "72d01fdd",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"Promise {\n",
" \u001b[36m<rejected>\u001b[39m TypeError: Expected string at position 1\n",
" at Object.readFile (ext:deno_fs/30_fs.js:716:29)\n",
" at <anonymous>:2:6\n",
" \u001b[36m<rejected>\u001b[39m NotFound: No such file or directory (os error 2): readfile ''\n",
" at async Object.readFile (ext:deno_fs/30_fs.js:716:18) {\n",
" name: \u001b[32m\"NotFound\"\u001b[39m,\n",
" code: \u001b[32m\"ENOENT\"\u001b[39m\n",
" }\n",
"}"
]
},
"execution_count": 36,
"execution_count": 16,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -566,13 +568,13 @@
},
{
"cell_type": "code",
"execution_count": 37,
"execution_count": 17,
"id": "28cf59d0-6908-4edc-bb10-c325beeee362",
"metadata": {},
"outputs": [
{
"data": {},
"execution_count": 37,
"execution_count": 17,
"metadata": {},
"output_type": "execute_result"
},
Expand All @@ -590,13 +592,13 @@
},
{
"cell_type": "code",
"execution_count": 38,
"execution_count": 18,
"id": "8d5485c3-0da3-43fe-8ef5-a61e672f5e81",
"metadata": {},
"outputs": [
{
"data": {},
"execution_count": 38,
"execution_count": 18,
"metadata": {},
"output_type": "execute_result"
},
Expand All @@ -614,7 +616,7 @@
},
{
"cell_type": "code",
"execution_count": 39,
"execution_count": 19,
"id": "1401d9d5-6994-4c7b-b55a-db3c16a1e2dc",
"metadata": {},
"outputs": [
Expand All @@ -624,7 +626,7 @@
"\u001b[32m\"Cool 🫡\"\u001b[39m"
]
},
"execution_count": 39,
"execution_count": 19,
"metadata": {},
"output_type": "execute_result"
}
Expand All @@ -635,13 +637,13 @@
},
{
"cell_type": "code",
"execution_count": 40,
"execution_count": 20,
"id": "7afdaa0a-a2a0-4f52-8c7d-b6c5f237aa0d",
"metadata": {},
"outputs": [
{
"data": {},
"execution_count": 40,
"execution_count": 20,
"metadata": {},
"output_type": "execute_result"
},
Expand All @@ -665,19 +667,24 @@
},
{
"cell_type": "code",
"execution_count": 1,
"execution_count": 21,
"id": "8e93df23-06eb-414b-98d4-51fbebb53d1f",
"metadata": {},
"outputs": [
{
"ename": "TypeError",
"evalue": "Cannot read properties of undefined (reading 'broadcast')",
"output_type": "error",
"traceback": [
"Stack trace:",
"TypeError: Cannot read properties of undefined (reading 'broadcast')",
" at <anonymous>:2:20"
]
"data": {
"text/html": [
"<b>Complete ✅</b>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

]
},
"metadata": {},
"output_type": "display_data"
},
{
"data": {},
"execution_count": 21,
"metadata": {},
"output_type": "execute_result"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a separate PR, we should make sure to not send an execute_result if we don't have any payload.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, that looks like a bug, we should be skipping this message if the only value is undefined.

],
"source": [
Expand Down
Loading