Skip to content

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou added the dependencies Pull requests that update a dependency file label Oct 13, 2022
@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch martinRenou/voila/nbclient_pin

@github-actions
Copy link
Contributor

github-actions bot commented Oct 13, 2022

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

Results table
Test file voila-tree-classic.ipynb voila-tree-light.ipynb voila-tree-dark.ipynb voila-tree-miami.ipynb basics.ipynb bqplot.ipynb dashboard.ipynb gridspecLayout.ipynb interactive.ipynb ipympl.ipynb ipyvolume.ipynb multiple_widgets.ipynb query-strings.ipynb reveal.ipynb
Render
chromium
actual 93 <- [104 - 119 - 157] -> 263 71 <- [74 - 81 - 99] -> 139 74 <- [81 - 81 - 97] -> 136 69 <- [75 - 84 - 100] -> 137 2440 <- [2490 - 2571 - 2664] -> 3268 2379 <- [2420 - 2441 - 2568] -> 2770 2655 <- [2665 - 2686 - 2806] -> 3133 2565 <- [2610 - 2641 - 2772] -> 3017 1921 <- [1945 - 1948 - 1950] -> 2233 3056 <- [3070 - 3173 - 3198] -> 3449 8350 <- [8447 - 8449 - 8477] -> 8485 6441 <- [6463 - 6486 - 6627] -> 6687 1374 <- [1389 - 1435 - 1505] -> 1526 2255 <- [2287 - 2317 - 2387] -> 2452
expected 3379 <- [3442 - 3517 - 3701] -> 3876 2976 <- [3227 - 3321 - 3421] -> 3604 3608 <- [3623 - 3709 - 3793] -> 3825 4453 <- [4453 - 4523 - 4661] -> 4748 2559 <- [2655 - 2656 - 2660] -> 2674 3982 <- [4079 - 4213 - 4356] -> 4743 12183 <- [18509 - 19553 - 20811] -> 21515 15319 <- [15660 - 15796 - 15912] -> 16056 1517 <- [1920 - 1997 - 2103] -> 2113

❗ Test metadata have changed
--- /dev/fd/63	2022-10-25 10:04:56.927950940 +0000
+++ /dev/fd/62	2022-10-25 10:04:56.927950940 +0000
@@ -4,37 +4,37 @@
     "BENCHMARK_REFERENCE": "actual"
   },
   "browsers": {
-    "chromium": "97.0.4666.0"
+    "chromium": "94.0.4595.0"
   },
   "systemInformation": {
     "cpu": {
-      "brand": "Xeon® Platinum 8272CL",
+      "brand": "Xeon® E5-2673 v3",
       "cache": {
         "l1d": 65536,
         "l1i": 65536,
-        "l2": 2097152,
-        "l3": 36700160
+        "l2": 524288,
+        "l3": 31457280
       },
       "cores": 2,
       "family": "6",
-      "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single pti fsgsbase bmi1 hle avx2 smep bmi2 erms invpcid rtm avx512f avx512dq rdseed adx smap clflushopt avx512cd avx512bw avx512vl xsaveopt xsavec xsaves md_clear",
+      "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm invpcid_single pti fsgsbase bmi1 avx2 smep bmi2 erms invpcid xsaveopt md_clear",
       "governor": "",
       "manufacturer": "Intel®",
-      "model": "85",
+      "model": "63",
       "physicalCores": 2,
       "processors": 1,
       "revision": "",
       "socket": "",
-      "speed": 2.6,
+      "speed": 2.4,
       "speedMax": null,
       "speedMin": null,
-      "stepping": "7",
+      "stepping": "2",
       "vendor": "GenuineIntel",
       "virtualization": false,
       "voltage": ""
     },
     "mem": {
-      "total": 7281311744
+      "total": 7291699200
     },
     "osInfo": {
       "arch": "x64",
@@ -42,11 +42,11 @@
       "codename": "Focal Fossa",
       "codepage": "UTF-8",
       "distro": "Ubuntu",
-      "kernel": "5.15.0-1022-azure",
+      "kernel": "5.8.0-1040-azure",
       "logofile": "ubuntu",
       "platform": "linux",
-      "release": "20.04.5 LTS",
-      "serial": "cb2c9040582e4276b4bc683d9e14354c",
+      "release": "20.04.3 LTS",
+      "serial": "cfc067bfcb844f35865e279a1b0e66c5",
       "servicepack": "",
       "uefi": false
     }

@jtpio jtpio added this to the 0.4.0 milestone Oct 13, 2022
@jtpio
Copy link
Member

jtpio commented Oct 13, 2022

Nice thanks!

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Diff looks good, waiting for the CI checks to complete.

Thanks!

@jtpio
Copy link
Member

jtpio commented Oct 13, 2022

ah the CI checks are still running, maybe there is some real issue underneath (timeout).

@jtpio
Copy link
Member

jtpio commented Oct 13, 2022

Cancelling and restarting the checks.

@jtpio
Copy link
Member

jtpio commented Oct 17, 2022

Doing one more full CI kick.

@jtpio jtpio closed this Oct 17, 2022
@jtpio jtpio reopened this Oct 17, 2022
@jtpio
Copy link
Member

jtpio commented Oct 17, 2022

cc @davidbrochart who might know if something needs to be updated to handle the latest nbclient.

@bnavigator
Copy link

So, I tried 0.4.0rc0 on the openSUSE Build Service with the same change as here and first thought I could not reproduce the hang form GitHub Actions, But then noticed that the pytest started to hang before the first test as soon as I had updated jupyter-client from 7.3.4 to 7.4.2. I suspect a non-returning test fixture.

@bnavigator
Copy link

Pinning jupyter-client got rid of the hangs: https://github.com/bnavigator/voila/actions/runs/3268022596

That's not a viable solution but might lead us into the right direction.

Ping @blink1073

@blink1073
Copy link

Note there is a significant refactor in the the forthcoming jupyter_client 8.0 to remove nest-asyncio and use public asyncio APIs.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Oct 18, 2022
https://build.opensuse.org/request/show/1029570
by user bnavigator + dimstar_suse
- Bump to nbclient 0.7 (See also gh#voila-dashboards/voila#1234)
- Remove import from vendorized mistune in nbconvert: not
  vendorized anymore
@trungleduc
Copy link
Member

Thanks @bnavigator for the pointer, indeed the changes of jupyter-client from 7.4.1 to 7.4.2 (trungleduc#6) cause the hanging tests. But strangely i can't reproduce the issue while running the GHA locally with https://github.com/nektos/act.

The diff between the two version is very short (https://github.com/jupyter/jupyter_client/pull/852/files) but i can not find any clue from the diff. Ping @davidbrochart who might understand what happened here.

@davidbrochart
Copy link
Member

I don't know, I see that @blink1073 backported a fix from the PR that removes nest-asyncio, that might have consequences.

@blink1073
Copy link

Based on the diff, I would suspect the handling of asyncio sockets in Session to be the culprit.

@bnavigator
Copy link

For the record, an update to jupyter_client 7.4.3 does not change anything here:

[   16s] + pytest-3.9 --ignore=_build. --ignore=_build.python39 --ignore=_build.python310 -v tests -k 'not (test_kernel_death or test_request_with_query)' --setup-show
[   16s] ============================= test session starts ==============================
[   16s] platform linux -- Python 3.9.14, pytest-7.1.2, pluggy-1.0.0 -- /usr/bin/python3.9
[   16s] cachedir: .pytest_cache
[   16s] rootdir: /home/abuild/rpmbuild/BUILD/voila-0.4.0rc0
[   16s] plugins: anyio-3.5.0, rerunfailures-10.2, tornasync-0.6.0.post2
[   17s] collecting ... collected 73 items / 6 deselected / 67 selected
[   17s] 
[   17s] tests/execute_output_test.py::test_execute_output 
[   17s]         SETUP    F sleep_between_tests
(...hangs indefinitely)

@bnavigator
Copy link

bnavigator commented Oct 23, 2022

So skipping test_execute_output lets the rest of the test suite pass.

I stepped through the test and it hangs somewhere here:

abuild@skylab:~/rpmbuild/BUILD/voila-0.4.0rc0> pytest --trace tests/execute_output_test.py 
===================================================== test session starts =====================================================
platform linux -- Python 3.10.7, pytest-7.1.2, pluggy-1.0.0
rootdir: /home/abuild/rpmbuild/BUILD/voila-0.4.0rc0
plugins: anyio-3.5.0, rerunfailures-10.2, tornasync-0.6.0.post2
collected 1 item                                                                                                              

tests/execute_output_test.py 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB runcall (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /home/abuild/rpmbuild/BUILD/voila-0.4.0rc0/tests/execute_output_test.py(39)test_execute_output()
-> path = os.path.join(BASE_DIR, 'notebooks/output.ipynb')
(Pdb) n
> /home/abuild/rpmbuild/BUILD/voila-0.4.0rc0/tests/execute_output_test.py(40)test_execute_output()
-> nb = read(path, NO_CONVERT)
(Pdb) n
> /home/abuild/rpmbuild/BUILD/voila-0.4.0rc0/tests/execute_output_test.py(41)test_execute_output()
-> nb_voila = deepcopy(nb)
(Pdb) n
> /home/abuild/rpmbuild/BUILD/voila-0.4.0rc0/tests/execute_output_test.py(42)test_execute_output()
-> executenb(nb_voila)
(Pdb) s
--Call--
> /usr/lib/python3.10/site-packages/voila/execute.py(130)executenb()
-> def executenb(nb, cwd=None, km=None, **kwargs):
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(131)executenb()
-> resources = {}
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(132)executenb()
-> if cwd is not None:
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(135)executenb()
-> nb, resources = ClearOutputPreprocessor().preprocess(nb, resources)
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(136)executenb()
-> executor = VoilaExecutor(nb, km=km, **kwargs)
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(137)executenb()
-> return executor.execute(nb, resources, km=km)
(Pdb) s
--Call--
> /usr/lib/python3.10/site-packages/voila/execute.py(54)execute()
-> def execute(self, nb, resources, km=None):
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(55)execute()
-> try:
(Pdb) n
> /usr/lib/python3.10/site-packages/voila/execute.py(56)execute()
-> result = super(VoilaExecutor, self).execute()
(Pdb) s
--Call--
> /usr/lib/python3.10/site-packages/nbclient/util.py(84)wrapped()
-> def wrapped(*args, **kwargs):
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(85)wrapped()
-> return just_run(coro(*args, **kwargs))
(Pdb) s
--Call--
> /usr/lib/python3.10/site-packages/nbclient/util.py(40)just_run()
-> def just_run(coro: Awaitable) -> Any:
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(42)just_run()
-> try:
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(43)just_run()
-> loop = asyncio.get_running_loop()
(Pdb) n
RuntimeError: no running event loop
> /usr/lib/python3.10/site-packages/nbclient/util.py(43)just_run()
-> loop = asyncio.get_running_loop()
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(44)just_run()
-> except RuntimeError:
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(45)just_run()
-> loop = None
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(46)just_run()
-> if loop is None:
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(47)just_run()
-> had_running_loop = False
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(48)just_run()
-> loop = asyncio.new_event_loop()
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(49)just_run()
-> asyncio.set_event_loop(loop)
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(52)just_run()
-> if had_running_loop:
(Pdb) n
> /usr/lib/python3.10/site-packages/nbclient/util.py(60)just_run()
-> return loop.run_until_complete(coro)
(Pdb) n
### hang ###

Maybe related to jupyter/nbclient#249, @davidbrochart ?

@martinRenou
Copy link
Member Author

Thanks a lot everyone for investigating! It looks like jupyter_client 7.4.2 was indeed the culprit, we will have an upper-bound for now and release 0.4.0.

@martinRenou martinRenou merged commit 7c0ed27 into voila-dashboards:main Oct 25, 2022
@martinRenou martinRenou deleted the nbclient_pin branch October 25, 2022 10:22
@jtpio
Copy link
Member

jtpio commented Oct 25, 2022

It looks like jupyter_client 7.4.2 was indeed the culprit

Does any of the 7.4.3 or 7.4.4 releases fix it?

@bnavigator
Copy link

bnavigator commented Oct 25, 2022

7.4.3 does not. Checking 7.4.4 now..

Update: No.

@bnavigator
Copy link

Still an issue with jupyter_client 7.4.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants