Alter parent settings to messages #738
Conversation
I don't understand quite how the parent message thing works, but setting the parent on every message breaks things. This is why the pyolite kernel breaks if you call await input twice, because it sets 'input_reply' as the parent message, rather than keeping the 'execute_request' there. I'm not convinced that this fix is perfect and that it doesn't need setting for other messages, but I thought it was worth throwing it out for discussion.
|
|
|
Fixes issue #650 |
|
This could just check if the message is 'input_response' and not update parent there. I'm not sure whether there are any other messages that need a parent message or not. |
|
I've just had a read through and I think parent header should only be set for execute requests for now. I think otherwise there is a risk that other messages will break the parent header stuff. I think if anything uses both execute and custom comms it will break, but that would happen now also. Really for it to work correctly, parent should be passed into the kernel execute request, which could then pass it out again, so you've always got the correct parent message. |
|
Thanks @joemarshall. The current handling of the parent messages is indeed not optimal at the moment. However it's not clear whether limiting it to |
| const msgType = msg.header.msg_type; | ||
| if (msgType === 'execute_request') { | ||
| this._parent = msg; |
There was a problem hiding this comment.
Could this be moved to this._execute maybe ?
I don't understand quite how the parent message thing works, but setting the parent on every message breaks things.
This is why the pyolite kernel breaks if you call await input twice, because it sets 'input_reply' as the parent message, rather than keeping the 'execute_request' there. With this fix, await input works fine.
I'm not convinced that this fix is perfect and that it doesn't need setting for other messages, but I thought it was worth throwing it out for discussion.