symmetryinvestments / jupyter-wire

Jupyter wire protocol implementation enabling D plugins to be jupyter kernels
Boost Software License 1.0
13 stars 4 forks source link

set null username and session fields to empty strings #31

Closed joequant closed 3 years ago

joequant commented 3 years ago

This fixes symmetryinvestments/jupyter-wire#27 in which jupyter-wire kernels will fail with jupyterlab. The issue is that the headers will have nulls rather than empty strings which results in the message sent to the frontend being None which will cause the frontend to not work.

codecov[bot] commented 3 years ago

Codecov Report

Merging #31 (601aba4) into master (722c2e5) will decrease coverage by 0.16%. The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   49.13%   48.97%   -0.17%     
==========================================
  Files           9        9              
  Lines         580      586       +6     
==========================================
+ Hits          285      287       +2     
- Misses        295      299       +4     
Impacted Files Coverage Δ
source/jupyter/wire/kernel.d 1.88% <0.00%> (-0.08%) :arrow_down:
source/jupyter/wire/message.d 34.21% <100.00%> (+1.77%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 722c2e5...601aba4. Read the comment docs.

skoppe commented 3 years ago

I think the approach in https://github.com/symmetryinvestments/jupyter-wire/pull/32 is preferable.

It is a shame asdf doesn't respect the default value, but overwrites it with null if not present. Otherwise the diff would be smaller still.

skoppe commented 3 years ago

I am going to close this in favor of #32