Github user kjmrknsn commented on the issue:
https://github.com/apache/zeppelin/pull/1302
The bug that "auto-restart interpreter on cron execution" restarts all
interpreters will be fixed by https://github.com/apache/zeppelin/pull/2681.
---
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Yeah, i think we can make "auto-restart interpreter on cron execution"
checkbox restart specific interpreter instance to the notebook.
---
Github user toughrogrammer commented on the issue:
https://github.com/apache/zeppelin/pull/1302
But isn't this original issue also important? I usually want to restart
specific notebook. Because spark executors aren't returned automatically, I
must kill zeppelin application from YARN
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Ah i see. That definitely looks like a problem. I filed an issue
https://issues.apache.org/jira/browse/ZEPPELIN-2995.
---
Github user toughrogrammer commented on the issue:
https://github.com/apache/zeppelin/pull/1302
https://user-images.githubusercontent.com/1473538/31558707-001fa9e6-b089-11e7-9ca6-48aabcc60839.png;>
I'm using spark interpreter per note isolated mode and I wrote a notebook
for
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@toughrogrammer Could you explain little more about the problem? How cron
execution is related with restart interpreter?
---
Github user toughrogrammer commented on the issue:
https://github.com/apache/zeppelin/pull/1302
How it's going on? I want to restart *note specific interpreter* when cron
is executed or refreshing spark context, but current version(0.7.2), it's
impossible. It's really unproductive
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@jongyoul I totally agree with you adding multi-tenancy but how about
separating that to another PR because we also already have restarting
interpreter function in the `Interpreter Setting
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Can you add consideration for the multi-tenancy? If Shiro is enabled,
current implementation will disable all of users' interpreters
---
If your project is set up for it, you can reply to this
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
re-trigger CI
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so,
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@corneadoug, @Leemoonsoo I re-based and added testcase. please review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@corneadoug I'm going to add test case soon. :laughing:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Can you rebase so that we can check the CI?
I will merge after it's green
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@Leemoonsoo I agree with your opinion. Let me fix them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim @corneadoug, Shell we have some unittests for `public void
restart(String settingId, String noteId)` before we merge? Restarting only a
interpreter instance/process for a note is
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Merging if there is no more discussions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@corneadoug I just re-based thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Tested LGTM
@astroshim can you rebase? It should make the CI green.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@corneadoug I fixed it. please review. Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Maybe a spinner just like the save button after editing an interpreter
would be enough then.
Its just a matter of having a small feedback.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Looks great to me, thank you @astroshim !
On @corneadoug point - it should be possible to animate the icon button \w
arrows to be i.e rotating while "restart is in progress" so it stops
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1302
My only feedback is that right now, it's hard to know if the interpreter
has finished restarting or not.
My guess is that it takes some time, so you shouldn't be able to restart it
again
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Tested again and it works well. I like it :)
Here is the new UI (for the other reviewers)
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@AhyoungRyu I think `deactivated` button is better so I updated it. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Looks good!
I think it would be better the activated and deactivated interpreter
buttons can be in line like below images.
- Before (with this patch)
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
ping
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
I just changed to 'icon'.
Thank you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Great! Personally I would prefer "just icon" like the reload
notes icon in Zeppelin home.
But it's just my personal opinion. Final decision is up to you :)
---
If your project
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Definitely looks better! I like @Leemoonsoo 's idea ð
It's just my personal opinion, how about putting `refresh` icon to the left
side of the interpreter name? I mean
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
\cc @Leemoonsoo I fixed UI like following screenshot and process. please
review.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@Leemoonsoo Your suggestion about UI is definitely better!
and also makes sense about restarting action on interpreter binding page.
I will fix them.
---
If your project is set up for
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1302
And i think it's more make sense and useful if we can make restart button
like,
restart button on interpreter setting page : restart all interpreter
instances/processes related to the
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Thanks for the contribution.
How about make UI little bit more consistent?
On interpreter setting page, we have interpreter restart button look like
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
Thank you very much about reviewing this PR. :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim Oh that was my bad. Please ignore my above comment. I only build
`zeppelin-web` not backend side.
Tested again and it works well as expected. Let's wait the other reviewers
:)
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/1302
@astroshim I've also thought the interpreter `restart` button in separated
page quite uncomfortable. This PR can be a good start for the better UX i think
:)
I tested your patch and it
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1302
ping
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
37 matches
Mail list logo
