Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1846
LGTM, merging if there's no more discussion
---
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 feat
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
ping @jongyoul @Leemoonsoo @felixcheung
---
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
e
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Tests LGTM. 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
enabled and wishes s
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
CI failure is not relevant.
```
Failed tests:
NotebookTest.testAbortParagraphStatusOnInterpreterRestart:760
expected: but was:
AngularElem
- should provide onclick method
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Thanks @Leemoonsoo Unit test is added. Please help review \cc @felixcheung
@jongyoul
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as wel
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1846
[AuthenticationIT.java](https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java)
is test that uses zeppelin with shiro
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
@Leemoonsoo Could you point me the right place to put this unit test? I
think this may need to enable shiro, but I didn't find any test with shiro
enabled.
---
If your project is set up for it,
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1846
@zjffdu Thanks for the patch!
I also think this PR deserves a unittest to make sure correct restart
behavior, like @felixcheung mentioned, if it is not too much difficult.
---
If your
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1846
@jongyoul agreen on refactoring Interpreter component, I also mention that
in other PRs. I'd love to help on that :)
---
If your project is set up for it, you can reply to this email and have your
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Sounds good!!
---
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 wishe
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1846
Good catch. LGTM @felixcheung We had too big InterpreterFactory,
InterpreterSetting and InterpreterGroup. I will refactor all of this class by
next major release - 0.8.0 - and will make them test
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/1846
oh gosh :)
is it possible to have test for this?
---
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 ha
12 matches
Mail list logo
