ZEPPELIN-346 Fix NotebookTest not passing non-default group to NoteIn…

[RPCMoritz]

…terpreterLoader

This implements the actually required logic for this test to pass and should mostly showcase that the NoteIntepreterLoader logic is flawed/broken. Nonetheless this would fix the test in the short term and be a step towards actually documenting the weird behaviour.

I will try to amend this PR (or create a separate one) to actually approach the underlying issue

[RPCMoritz]

@Leemoonsoo if you plan to release a new version, then you should probably merge this as-is.
A fix of the underlying logic is going to be more involved and won't happen too soon - I'll still attempt it though.

[Leemoonsoo]

@RPCMoritz Thanks for making the patch and interest to Zeppelin.

Selecting interpreter using %[group].[name] notation introduced by https://issues.apache.org/jira/browse/ZEPPELIN-74. Main reason the logic working in this way is to keep compatibility with previous behavior, ie. to keep selecting interpreter simple.
Guessing interpreter from partial string (%[group] or %[name]) is intended behavior.
Only confusion zeppelin does not handle right now is when there're '.' (dot) in [group] or [name] string.

So, I'd like to understand more about why do you think there're bad test and ambiguous logic, and what are you trying to fix in the end. Could you explain more?

[RPCMoritz]

I believe the test currently only passes CI, because of the broken implementation of using System properties. Without setting group.name explicetely, the guessing code does not correctly guess the second interpreter by name only, because the structure used does not match how it's written into system properties (in this test). The actual issue may be in the module writing system properties, but I assert that using System properties in this way is not ideal, and using API calls or a dedicated configuration object instead is more useful (but still not generally independent across multiple notebooks and therefore influenced by tests being run in parallel)

So the current fix to the test makes tests pass in linear execution.
My proposed fix of the actual logic is to either
a) fix writing/parsing System properties
or
b) move to an object oriented design and remove the use of System properties for internal variables, if possible.

I looked at this issue together with @FRosner , who gave his commentary in the JIRA issue, which may be helpful in making my point.

[Leemoonsoo]

Could you help me to reproduce the test error? Because of the test also passes in my machines, both using mvn test command and run inside of IDE.

[RPCMoritz]

On any machine I attempted to build, I got the test failure (interpreter not found). I'll try again with the current master on yet another machine to see if I can still reproduce.

[FRosner]

@Leemoonsoo the test also failed on my machine at the time the patch was proposed.

I still think, however, that passing two parameters as one by splitting a String is a very bad idea. See https://issues.apache.org/jira/browse/ZEPPELIN-346?focusedCommentId=14960634&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14960634

[bzz]

@RPCMoritz@FRosner does the issue still exist on the latest master?

[RPCMoritz]

I will re-test on my systems ASAP.

[RPCMoritz]

testSelectingReplImplementation(org.apache.zeppelin.notebook.NotebookTest) Time elapsed: 0.032 sec <<< ERROR!
org.apache.zeppelin.interpreter.InterpreterException: mock2 interpreter not found
at org.apache.zeppelin.notebook.NoteInterpreterLoader.get(NoteInterpreterLoader.java:148)
at org.apache.zeppelin.notebook.Note.run(Note.java:365)
at org.apache.zeppelin.notebook.NotebookTest.testSelectingReplImplementation(NotebookTest.java:119)

with

Apache Maven 3.2.5 (12a6b3acb947671f09b81f49094c53f426d8cea1; 2014-12-14T18:29:23+01:00)
Maven home: /usr/share/maven-bin-3.2
Java version: 1.7.0_80, vendor: Oracle Corporation
Java home: /opt/oracle-jdk-bin-1.7.0.80/jre
Default locale: en_US, platform encoding: ISO-8859-15
OS name: "linux", version: "3.16.5-gentoo", arch: "amd64", family: "unix"

So, still broken.