Fix #262: JSONErrorsMixin not returning correct results

[kevin-bates]

After Notebook 5.2.0 is installed, JSONErrorsMixin.write_error is
used to convert errors into JSON responses. Previously, Notebook's
@json_errors decorator performed this action. Since the decorator's
replacement, APIHandler.write_error, doesn't adequately set the
reason field, other tests fail so its not sufficient to drop the
use of JSONErrorsMixin. As a result, JSONErrorsMixin now incorporates
the appropriate code from APIHandler.write_error (in particular the
setting of the traceback field) - in addition to appropriately
setting the reason field. This mixin should continue to be used
in the handler class hierachy until APIHandler is fixed.

I have tested these changes with Notebook 5.1.0 and 5.2.0 installed.

Fixes #262.

[rolweber]

I currently don't have time to dig deeper into the problem, but I had a look at the diff, and it looks good.

[parente]

Thanks for digging into this issue. It's not the first time we've found we've found an incompatibility with new notebook versions because KG is definitely touches more than the public API of the notebook server.

Is notebook>=5.2.0 now a requirement after this patch goes in? Or will it work with notebook 5.0 and up? I'm asking to see if we need to (1) bump the required minimum notebook version to 5.2 now and (2) see if this should be a patch release or a minor release, assuming we're sticking with semantic versioning.

[kevin-bates]

Hi @parente. No, this update doesn't introduce any dependency change on notebook versions. When notebook 5.1.0 (or less) is in play @json_errors builds the json exceptions, while with notebook 5.2.0, the JSONErrorsMixin builds the exception. All nosetests pass with this change and either version of notebook.

In addition, once the notebook issue with APIHandler.write_error is fixed (to set the reason field), then JSONErrorsMixin could be removed as a subclass on the various JKG handlers. The tests that call the mixin directly could either be removed or the class could remain (not sure if its needed at that point).

[parente]

Sorry for the delay on this one. I can work on backporting this to the 2.0.x branch to cut a 2.0.2 release with it soon.

[rolweber]

@parente: Can you find some time for cutting a new release?

[parente]

@rolweber @kevin-bates I've tagged 2.0.2 and pushed a release to pypi.

[kevin-bates]

@parente - thank you! Does something similar need to happen for anaconda envs? (Sorry, total newbie to this python stuff). Since our tests run via anaconda envs it still looks like they're picking up 2.0.1 from conda-forge.

jupyter_kernel_gateway:             2.0.1-py_0         conda-forge

[parente]

@kevin-bates Indeed. I'll cc on the PR over in the conda-forge feedstock.

[kevin-bates]

Awesome. Thank you @parente!