extract send_execute_reply method from Kernel.execute_request

[kmike]

This is related to #7614: I want do_executemethod to be async. Currently the whole execute_request should be copy-pasted to implement it; after this refactoring it is possible to return a deferred from do_execute and add a callback to it in overridden send_execute_reply which calls parent's send_execute_reply.

Example:

class MyKernel(Kernel):

    # ...

    def send_execute_reply(self, stream, ident, parent, md, reply_content):
        def done(result):
            return super(MyKernel, self).send_execute_reply(stream, ident, parent, md, result)
        assert isinstance(reply_content, defer.Deferred)
        reply_content.addCallback(done)

    def do_execute(self, code, silent, store_history=True, user_expressions=None,
                   allow_stdin=False):

        def publish_result(text):
            if not silent:
                stream_content = {'name': 'stdout', 'text': text}
                self.send_response(self.iopub_socket, 'stream', stream_content)

        def success(result):
            text = self.to_text(result)
            publish_result(text)
            return {
                'status': 'ok',
                'execution_count': self.execution_count,
                'payload': [],
                'user_expressions': {},
            }

        def error(failure):
            text = str(failure)
            publish_result(text)
            return {
                'status': 'error',
                'execution_count': self.execution_count,
                'ename': '',
                'evalue': text,
                'traceback': []
            }

        d = self.run_async(code)
        d.addCallbacks(success, error)
        return d

[takluyver]

Naming point: 'execute output' implies something a bit different. I would call this _flush_stdstreams.

[kmike]

It is specific to execute command because it sleeps for _execute_sleep delay, that's why I added 'execute' to the name. But there are other places where this function can be useful - should they also sleep? If so, I think it could make sense to rename _execute_sleep.

[minrk]

Does this logic suggest that all replies should be separated from their handler methods, rather than just execute replies?

[takluyver]

stop_on_error and silent are only used once below, it's probably not necessary to assign them to variables again.

[kmike]

I'd even add another variable for reply_msg['content']['status'] == u'error' to make the condition below nicer :) I'll remove it if you want.

[takluyver]

In principle I think this is fine.

[kmike]

@minrk a good point. Generally, yes; it is just that copy-paste savings are smaller for other methods.

[ellisonbg]

Do you plan on working on this soon? If not, do you mind if we close it until work restarts? We are doing major refactoring in the repo (our "Big Split") and existing PRs are going to bit-rot quickly (already).

[kmike]

Hi @ellisonbg,

No, I don't have plans to work on it soon. I'll open a new PR if I have time in future.

For the record: the current hack-ish solution lives here; there is one more problem with async methods which is worked around there ("idle" event was published too early).