Option to restart interpreter from paragraph.

[prabhjyotsingh]

In case of operation timeout, or any event in which user might think a restart to interpreter might fix the error in paragraph. A shortcut to restart that specific interpreter from the console, without going to the setting menu.

[Leemoonsoo]

I think providing a button to restart interpreter in the notebook page is good idea.

But 'Restart Interpreter' button seem to displayed when paragraph status is ERROR. However, paragraph can be ERROR status with such things like syntax error, table not found etc. So ERROR status does not always mean Interpreter restart is required.

And can think little bit more about position of the restart interpreter button and message? Is there any alternative place? because of interpreter is notebook specific. I think showing button in notebook is more intuitive than showing button in paragraph.

[prabhjyotsingh]

Yes, that does make sense.

How about if we try to look for string/error related to transport or texception (thrift) and then show it.

And giving restart at a notebook level makes an impression (to me) that there is problem with the whole notebook and after restart I need to re-run the entire notebook.

[prabhjyotsingh]

Have modified logic to show restart button only there is "timed out" or "thrift" related exception.

[r-kamath]

@prabhjyotsingh rebase

[prabhjyotsingh]

Rebased with master.

[felixcheung]

Could you please rebase again.
@Leemoonsoo @corneadoug comments?

[prabhjyotsingh]

Rebased with master.

[corneadoug]

There is a few values or function, that seems out of place to me, for example:
$scope.paragraph.interpreterRestarting should probably not be at paragraph level, but at Notebook level since interpreters are linked to Notebook. (and multiple paragraphs might share the same interpreter)
But since I don't have good solutions to re-organize the code better (function and communication between controllers - wise), I think its fine.

However, for the $scope.checkErrorForRestart(), it is usually bad for performances to link a function to a ng-show/hide or ng-if. Sometimes, it seems inevitable, however in this case we could probably assign a value to this error and change if when we process the query result.

[prabhjyotsingh]

@corneadoug Yes, I did have a thought about of having $scope.paragraph.interpreterRestarting at Notebook level, but then it more suited here.

For checkErrorForRestart(), I thought about it, but then its (paragraph.status == 'ERROR') && checkErrorForRestart(). So, only when there is an error this will get called, so it think it should be ok.

[corneadoug]

We can keep it like that for now

[felixcheung]

I guess from the usability point of view, would it be confusing - because restarting interpreter at a particular paragraph, but all local variables would be cleared right? The user would need restart running from the top? And not just re-run this paragraph?

[prabhjyotsingh]

@felixcheung I know it can be confusing (or ambiguous if run all was used), and yes I agree if restart, it may clear local variables depending upon interpreter (in case of spark yes).

So, will it make sense if we alter this message https://github.com/apache/incubator-zeppelin/pull/480/files#diff-52fa6628f0b6793d22ffe1f3e4763635R2110

[felixcheung]

@prabhjyotsingh probably - we could say 'restart this interpreter X - note that all interpreter states and local variables will be reset' or similar. Would be good to name the interpreter too, to be clear.

[prabhjyotsingh]

@felixcheung Sure, that's definitely a better idea, implemented that.
Also, moved restartInterpreterSetting function to notebook.controller.js

[felixcheung]

@Leemoonsoo @corneadoug any more thought on this?

[Leemoonsoo]

I still think there should be better place to put "interpreter restart" button, than the inside of paragraph.
Let's say, user run multiple paragraphs. They all have connection failure. and they all show "interpreter restart" button under. User click one of them, and rerun paragraph, but the other buttons in the other paragraphs will still remain there. Isn't it make some confusions?

Another thing is, how it detects the error.
Current detection routine search for one of "timed out", "thrift", "except", "connection refused" string from error message. But they're quite common words to be shown in any other error messages and we can not really sure it's error that requires restart.

What do you think??

[felixcheung]

agreed, and "except" would be rather common.
perhaps @corneadoug would have an suggestion on where to put a button or a dialog or something?

[corneadoug]

Sorry,
I had a pile of notifications.
It depends on how the error is detected, the best could be to fire the modal on error (if multiple errors, they check if the modal is fired or not), which would suppress the button and be only the modal shown before.

The only problem overall would be the timing of those errors (in case you already closed the modal or restarted the interpreter)