search

Coder-Spirit / Jupyter-PHP

A PHP Kernel for Jupyter
MIT License
221 stars 33 forks source link

Fix handling of multiple ZMQ identifiers #20

Closed thomasjm closed 7 years ago

thomasjm commented 7 years ago

A ZMQ message may contain more than 1 ZMQ identifier; see the section on the Jupyter wire protocol here: https://jupyter-client.readthedocs.io/en/latest/messaging.html#the-wire-protocol.

(This doesn't cause a problem with most Jupyter frontends, but I'm working on one it does cause an issue for.)

castarco commented 7 years ago

Hi, I'll review (and manually test) this PR during the day. Meanwhile, which clients are having problems with the current kernel?

Thank you!

thomasjm commented 7 years ago

My own client :) you can see it here: https://codedown.io/

(Can't seem to figure out how to make my ZMQ library use only a single identifier, so instead I've been submitting PRs to all the language kernels to make them conform to the spec :P)

thomasjm commented 7 years ago

Thanks! Would it be possible to cut a release with this fix?

(Did you mean to merge without changing the array() to [] ?)

castarco commented 7 years ago

Hi, yes, I merged it before the change because this can be fixed in a different PR :) . Your change was interesting enough to ship a new version.

By the way, there is a new release (0.1.7), but to install it you should use the installer with its install command (the update command has some problems).

castarco commented 7 years ago

Hi @thomasjm , was enough this PR to make this kernel production-ready for your platform? In the meantime I've been improving the installer a little bit.

thomasjm commented 7 years ago

Yes, it seems to work fine, thanks! (I haven't tested it exhaustively.)