notebook icon indicating copy to clipboard operation
notebook copied to clipboard

Handle execution errors with empty traceback entries similar to Lab

Open kevin-bates opened this issue 1 year ago • 11 comments

Some kernels (like Apache Toree) don't always set a traceback entry when returning errors. An example of this is an immediate syntax error that occurs for code like sdfsf. In such cases, Notebook does not render any output for the error, toree-current-nb yet Jupyter Lab does... toree-current-lab

In analyzing both the message responses and code bases, it turns out that Lab will display either the traceback, if it has a value, or the composition of the ename and evalue entries, while Notebook only displays the traceback entry (if there's a value).

This pull request updates Notebook's 6.4.x branch with equivalent code to Lab - rendering similar output. (Note that the second cell's output is produced from a non-empty traceback.) toree-fix-nb

(Heads up. I'm not a javascript dev and tried to follow current formatting and practices - so please feel free to correct anything that is superfluous or improper.)

kevin-bates avatar Jul 07 '22 20:07 kevin-bates

Hi @kevin-bates I am happy to review this. From last discussions, such changes should occur in the nbclassic repository. The plan is then to create a 6.5.x branch that will pull the static assets (javascript, css...) from nbclassic. We were not aiming anymore to cut a release from 6.4 branch.

It will still need a few more weeks to get a published 6.5 notebook release. Do you have time for that?

echarles avatar Jul 08 '22 08:07 echarles

Hi @echarles - thanks.

Do you have time for that?

I suppose I need to better understand the implications here before answering.

  1. Doesn't the move to nbclassic imply deployment of Jupyter Server rather than Notebook?
  2. Does the nbclassic configuration support all existing notebook extensions (nbextensions)?
  3. Does the nbclassic configuration support all existing server extensions? (Seems like, at a minimum, these would need to require updates to point at jupyter_server rather than notebook - or is there some shim in place.)

Deploying a different server is a non-trivial undertaking in larger organizations so I suppose the answer to your question would be "yes, there's time for that" due to all the deployment and configuration changes this implies (although I'm not sure they will want to "go there" just yet).

Not being familiar with the front-end pieces, would it be possible to "patch" the OutputArea.prototype.append_error function within a 6.4 configuration?

Finally, should I manually apply the same changes to https://github.com/jupyter/nbclassic/blob/main/nbclassic/static/notebook/js/outputarea.js or is this something MeeseeksDev can handle?

Thanks for your help.

kevin-bates avatar Jul 08 '22 14:07 kevin-bates

I suppose I need to better understand the implications here before answering.

For clarity, I was meaning "do you have time to wait for that". We (@RRosio @ericsnekbytes and I will do the work to create that 6.5.x branch)

Doesn't the move to nbclassic imply deployment of Jupyter Server rather than Notebook?

The move is done. If you install nbclassic, you will run on jupyter-server (with all what it means).

Does the nbclassic configuration support all existing notebook extensions (nbextensions)?

NbClassic supports the existing notebook extensions, thx to a shim merged and released last weekend.

All extensions? Not 100%, but close to that. We will only know when some users will open issues.

Does the nbclassic configuration support all existing server extensions? (Seems like, at a minimum, these would need to require updates to point at jupyter_server rather than notebook - or is there some shim in place.)

Not sure to fully understand you point. nbclassic is just another server extensions. If you install nbclassic, jupyterlab, notebook7, they will all run on the same server (same http port) without conflict, (we may implement some arbitrage, ideally in jupyter-server, for handlers competing for the same endpoint like "/tree").

Deploying a different server is a non-trivial undertaking in larger organizations so I suppose the answer to your question would be "yes, there's time for that" due to all the deployment and configuration changes this implies (although I'm not sure they will want to "go there" just yet).

I see you point. We can always rediscuss the no-new-release for 6.4.x during the next community call if you join to present your case :)

Not being familiar with the front-end pieces, would it be possible to "patch" the OutputArea.prototype.append_error function within a 6.4 configuration?

Maybe yes, with a notebook extension. I let you try this out, but it sounds to me worth doing that.

Finally, should I manually apply the same changes to https://github.com/jupyter/nbclassic/blob/main/nbclassic/static/notebook/js/outputarea.js or is this something MeeseeksDev can handle?

This could be done manually. @RRosio can say something about MeeseeksDev

Thanks for your help.

👍

echarles avatar Jul 08 '22 14:07 echarles

@kevin-bates To make it clear, if you wait for 6.5.x branch, you will run on the classic Tornado system, not on jupyter-server. Only the static assets (javascript, css, html) from nclassic will be used by notebook 6.5.x. The python code will be the same as 6.4.x

echarles avatar Jul 08 '22 14:07 echarles

Thanks @echarles

To make it clear, if you wait for 6.5.x branch, you will run on the classic Tornado system, not on jupyter-server. Only the static assets (javascript, css, html) from nclassic will be used by notebook 6.5.x. The python code will be the same as 6.4.x.

Hmm. When I install nbclassic I don't see notebook installed, only jupyter_server and notebook-shim. So by this comment: "The python code will be the same as 6.4.x." are you really saying the python code is "equivalent in functionality" to 6.4.x but is jupyter_server 1.x?

Does notebook-shim prevent the need for existing notebook server extension code to update their package references from notebook to juptyer_server? Looking at the site-packages, I don't see a notebook directory, so I suspect not. It seems like the notebook-shim package is primarily handling "configurable trait migration", but potential package references within an existing notebook extension would need to be updated to point into jupyter_server.

kevin-bates avatar Jul 08 '22 15:07 kevin-bates

Hmm. When I install nbclassic I don't see notebook installed, only jupyter_server and notebook-shim.

Correct, nbclassic does not depend on notebook anymore.

So by this comment: "The python code will be the same as 6.4.x." are you really saying the python code is "equivalent in functionality" to 6.4.x but is jupyter_server 1.x?

I was meaning the notebook 6.5.x python will be the same as the notebook 6.4.x (just forget about nbclassic in this context). So it is not jupyter_server 1.x.

Does notebook-shim prevent the need for existing notebook server extension code to update their package references from notebook to juptyer_server? Looking at the site-packages, I don't see a notebook directory, so I suspect not. It seems like the notebook-shim package is primarily handling "configurable trait migration", but potential package references within an existing notebook extension would need to be updated to point into jupyter_server.

The notebook-shim is there for nbclassic and aims to shim the configs. The shim merged last weekend to support the nbextensions on nbclassic is baked into nbclassic itself.

echarles avatar Jul 08 '22 15:07 echarles

@echarles and I just met and Eric clarified things for me and explained the 6.5.x efforts. This is amazing work you all have done and is really appreciated! It is not an easy task, to say the least.

I have opened https://github.com/jupyter/nbclassic/pull/126 and we'll move forward from there. Please feel free to close this PR if necessary.

kevin-bates avatar Jul 08 '22 16:07 kevin-bates

@echarles and I just met and Eric clarified things for me and explained the 6.5.x efforts. This is amazing work you all have done and is really appreciated! It is not an easy task, to say the least.

Thx @kevin-bates, really appreciate. Thx also for your comprehension and patience. We will review the https://github.com/jupyter/nbclassic/pull/126 begin next week and hope to get a 6.5.x notebook in the coming weeks.

echarles avatar Jul 08 '22 16:07 echarles

nbclassic 0.4.3 is released https://github.com/jupyter/nbclassic/releases/tag/v0.4.3 with the port of this fix. We still need to create a 6.5.x branch that will use nbclassic UI. @kevin-bates In the meantime, you may confirm that the fix works fine for you with the released nbclassic. Thx!

echarles avatar Jul 12 '22 06:07 echarles

@kevin-bates In the meantime, you may confirm that the fix works fine for you with the released nbclassic.

Indeed it does work fine. Thank you for driving this @echarles!

kevin-bates avatar Jul 12 '22 14:07 kevin-bates

@kevin-bates notebook 6.5.0b0 is released with this fix (being implemented as part of nbclassic frontend).

echarles avatar Aug 04 '22 04:08 echarles

This fix was included in nbclassic 0.4.3 which is used in Notebook 6.5. Closing.

kevin-bates avatar Oct 26 '22 20:10 kevin-bates