JSONErrorsMixin not returning correct results after Notebook 5.2.0 is installed #262
Comments
kevin-bates
added a commit
to kevin-bates/kernel_gateway
that referenced
this issue
Oct 18, 2017
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.
parente
added a commit
that referenced
this issue
Oct 31, 2017
Fix #262: JSONErrorsMixin not returning correct results
parente
pushed a commit
that referenced
this issue
Nov 11, 2017
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is related to Notebook Issue 2942.
One of the changes in Notebook 5.2.0 was the removal of the
@json_errorsdecorator via PR 2853. Instead, much of the equivalent code was moved into theAPIHandler.write_errormethod. However, by virtue of the decorator's removal and the inheritance hierarchy, theJSONErrorsMixin.write_errormethod is called instead ofAPIHandler.write_error.JSONErrorsMixin.write_errordoesn't handle the traceback functionality present in@json_errors(orAPIHandler) and results in the breakage of theTestCustomDefaultKerneltest.Since
APIHandler.write_errorshould be doing the equivalent of@json_errors, I commented out the subclassing ofJSONErrorsMixinfrom some of the handler classes, but then encountered a different set of test errors - these relating to noreasonfield on HTTP errors. It turns out that one of the pieces of functionality in@json_errorsnot "transferred" toAPIHandlerwas the appropriate setting of the reason code.Since its not sufficient to fallback to using
APIHandler, we should make the appropriate changes toJSONErrorsMixin.write_errorin order to bridge the gap between Notebook 5.2 GA and the future release that addresses the APIHandler/reason issue.The text was updated successfully, but these errors were encountered: