Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InterruptReply missing status and error properties #1100

Closed
garlandz-db opened this issue Feb 24, 2023 · 6 comments
Closed

InterruptReply missing status and error properties #1100

garlandz-db opened this issue Feb 24, 2023 · 6 comments

Comments

@garlandz-db
Copy link
Contributor

garlandz-db commented Feb 24, 2023

According to the jupyter messaging docs, we should be passing a status field in the content of the InterruptReply message. However in the implementation, we are just forwarding the request content (empty) back into the reply. As such, the reply has no information on status and the error properties in case the interrupt is not successful.

My suggestion on the fix should be a try catch around the send_interrupt_children and return a { 'status' : 'ok' } if no catch occurs and otherwise { 'status': 'error', ... }.

@blink1073
Copy link
Contributor

Good catch! Would you like to submit a PR?

@jasongrout-db
Copy link

It would be good to add a check in the tests at

async def test_direct_interrupt_request(kernel):
reply = await kernel.test_shell_message("interrupt_request", {})
assert reply["header"]["msg_type"] == "interrupt_reply"

async def test_direct_interrupt_request(ipkernel):
reply = await ipkernel.test_shell_message("interrupt_request", {})
assert reply["header"]["msg_type"] == "interrupt_reply"

(and to change those tests to send the interrupt request on the control channel instead of the shell channel).

@garlandz-db
Copy link
Contributor Author

i can get a PR in next week @blink1073.

@blink1073
Copy link
Contributor

Great, thanks!

@garlandz-db
Copy link
Contributor Author

any idea where i can get the traceback?

@blink1073
Copy link
Contributor

I would set a debug point in the tests and inspect the reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants