search

jupyter / atom-notebook

[Deprecated] Jupyter Notebook, but inside Atom.
MIT License
306 stars 48 forks source link

Remove reliance on netstat / lsof to find an open port #19

Closed monkpit closed 8 years ago

monkpit commented 8 years ago

Instead of parsing system calls to find an open port, an alternative implementation could use something like the portfinder package to find an open port regardless of the platform we are running on.

I initially referenced this issue in #16 as a niggle over using lsof, which of course doesn't work on Windows. The workaround was to use netstat for a more platform-independent solution, however it still relies upon grep (which will fail on most Windows systems), and the whole situation feels hacky to me.

We can avoid the whole try {systemCall} catch {doWork} pattern if we use something like portfinder.

For a trivial example, see https://github.com/monkpit/atom-notebook/commit/db43af5da891045093b66cdd405a30dd8f240be5 . I'm sure the error handling could be made much more robust, I'm open to any suggestions :+1: :smile:

gnestor commented 8 years ago

Much better solution @monkpit! The only issue with your PR was this.kernelGateway was bound to portfinder so that when the notebook is closed and the NotebookEditor class destroyed, the this.kernelGateway.stdin.pause() and this.kernelGateway.kill() were undefined and the process was not killed.