-
Notifications
You must be signed in to change notification settings - Fork 505
METRON-1382 Run Stellar in a Zeppelin Notebook #884
Conversation
nickwallen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few notes to guide reviewers...
| /** | ||
| * Provides auto-completion for Stellar. | ||
| */ | ||
| public class DefaultStellarAutoCompleter implements StellarAutoCompleter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class provides the core auto-completion logic for both the CLI-based REPL and Zeppelin.
| this.autocompleteIndex = initializeIndex(); | ||
|
|
||
| // TODO is this needed? FunctionResolver functionResolver | ||
| // // asynchronously update the index with function names found from a classpath scan. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this logic needed any longer? I don't feel it is, but need to dig on this some more.
| * Default implementation of a StellarShellExecutor. | ||
| */ | ||
| public class DefaultStellarShellExecutor implements StellarShellExecutor { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class provides the core execution logic for both the REPL and Zeppelin. It handles core Stellar in addition to the "extensions" like assignment, magics, docstrings, etc.
| for(SpecialCommand command : commandRegistry) { | ||
| if(command.getMatcher().apply(expression)) { | ||
| return command.execute(expression, this); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anything that is not "core" Stellar is defined separately as a SpecialCommand.
| new MagicDefineGlobal(), | ||
| new MagicUndefineGlobal(), | ||
| new MagicListGlobals() | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the specials are made available here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we ever do something like add assignment to the core Stellar language (like #687) then all we have to do is not use/remove the AssignmentCommand class.
|
|
||
| public interface SpecialDefinedListener { | ||
| void whenSpecialDefined(SpecialCommand magic); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event listeners can be notified when variables, functions, or specials are defined. Useful in our case for notifying the autocompleter.
| * | ||
| * Useful for debugging Stellar expressions. | ||
| */ | ||
| public class StellarShell extends AeshConsoleCallback implements Completion { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The StellarShell now only contains logic to drive the Aesh console.
| */ | ||
| @Override | ||
| public void complete(CompleteOperation completeOperation) { | ||
| String buffer = completeOperation.getBuffer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ties our generic autocompleter to the autocomplete interface required by Aesh.
| * @param executor A stellar execution environment. | ||
| * @return The result of executing the magic command. | ||
| */ | ||
| StellarShellResult execute(String expression, StellarShellExecutor executor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interface for all specials; assignment, comments, magics, docstrings, etc.
|
|
||
| [Apache Zeppelin](https://zeppelin.apache.org/) is a web-based notebook that enables data-driven, interactive data analytics and collaborative documents with SQL, Scala and more. This project provides a means to run the Stellar REPL directly within a Zeppelin Notebook. | ||
|
|
||
| ## Installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instructions for installing/running in a notebook.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add the license header to this? #884 is close to going in and enforcing this, so I'm hoping to avoid impact to master.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done.
26f6558 to
790cdd5
Compare
nickwallen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More notes to help guide reviewers through the code.
| data[i++] = new String[] { toWrappedString(kv.getKey().toString(), wordWrap) | ||
| , toWrappedString(result.getResult(), wordWrap) | ||
| , toWrappedString(result.getExpression(), wordWrap) | ||
| , toWrappedString(result.getExpression().get(), wordWrap) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VariableResult.expression field is now optional. We do not always know the expression that resulted in a value. The ShellFunctions just needed updated to treat this as an Optional.
| ``` | ||
| $ mvn exec:java \ | ||
| -Dexec.mainClass="org.apache.metron.stellar.common.shell.StellarShell" \ | ||
| -Dexec.mainClass="org.apache.metron.stellar.common.shell.cli.StellarShell" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StellarShell, the main driver class for the CLI-based REPL, was moved to its own package since it is only used by the CLI-based REPL. This separates it from the other core classes that are used by both the Zeppelin and CLI-based REPLs.
| new MagicDefineGlobal(), | ||
| new MagicUndefineGlobal(), | ||
| new MagicListGlobals() | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we ever do something like add assignment to the core Stellar language (like #687) then all we have to do is not use/remove the AssignmentCommand class.
| */ | ||
| public interface FunctionDefinedListener { | ||
| void whenFunctionDefined(StellarFunctionInfo functionInfo); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used an event listener pattern so that external entities could get notified when things occur during the execution of Stellar. For example, the auto-completer needs to know when a variable is defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing seems to be ever removed, can that ever be a problem? That will typically cause retention... and 'leaks'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this would cause a leak. Yes, in some cases listeners can cause troubles, but I don't see how it is a problem here. Can you be more specific? Otherwise, I don't know what needs a fixin' here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, battle scars.
| * Add a listener that will be notified when a magic command is defined. | ||
| * @param listener The listener to notify. | ||
| */ | ||
| void addSpecialListener(StellarExecutionListeners.SpecialDefinedListener listener); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our DefaultStellarShellExecutor is a StellarExecutionNotifier as it is able to notify event listeners when variables, functions or specials are defined.
| * | ||
| * For example `?TO_STRING` will output the docs for the function `TO_STRING` | ||
| */ | ||
| public class DocCommand implements SpecialCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc strings are not part of the core language.
| * | ||
| * quit | ||
| */ | ||
| public class QuitCommand implements SpecialCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what allows a user to execute quit within the REPL. Again, not part of core Stellar.
| * This is typically an action performed on the execution | ||
| * environment, not something that could be executed within Stellar. | ||
| */ | ||
| public interface SpecialCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interface that I used for any logic that we had implemented in the REPL that is not part of the Stellar language.
| /** | ||
| * A Zeppelin Interpreter for Stellar. | ||
| */ | ||
| public class StellarInterpreter extends Interpreter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what allows us to run Stellar in a Notebook.
| } | ||
|
|
||
| @Override | ||
| public List<InterpreterCompletion> completion(String buf, int cursor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where we connect our generic auto-completer that is used across both REPLs into the auto-complete extension point offered by Zeppelin.
|
This is quite a bit to go over.... can I trade you a similarly large review task? |
|
I think this is an excellent start. So far I have only reviewed it from a user perspective and it's working well. I've spun it up in full dev (not sure that's even necessary) and installed this via the instructions in the README. I was able to run most of the examples in the Stellar README. I found that the functions available in Zeppelin are a subset of what's available in the Stellar shell. The missing functions include IS_EMAIL, ENRICHMENT*, GEO*, STATS* and many others. Is this expected? Is there a way to pass in the zookeeper url in Zeppelin? |
|
I also noticed that functions returning a list only display the first item in both the shell and Zeppelin output. For example, the expression From what I can tell the problem is only in how the result is displayed. For example, the expression |
|
Thanks for digging into it @merrimanr . You uncovered some good stuff.
Yes, since we only added I just updated the README to clarify this point. I also added instructions for adding additional libraries to gain access to more Stellar functions. |
No, I did not implement that in this PR. As a next step I was going to do whatever needs done to get the management functions working in Zeppelin. That would include adding a Zookeeper URL. |
BUG! Good find. The value getting returned is always the first. I'll fix that. |
|
The problem is on the UI side of things; for both Zeppelin and the CLI. When I get the result back from Stellar, I was using |
|
@merrimanr Fixed that bug and was able to add a good amount of unit tests around it. Also figured out a way to unit test the Aesh-driven StellarShell class. |
…tion instructions in the README
|
Ok will do @ottobackwards |
|
And done. Please take a look when you get a chance. |
|
Is there a 'supported' or tested version of zeppelin? I am getting exceptions starting the stellar interpreter Also: In the zeppelin site file, there is already a zeppelin.interpreters property, which in a comma separated list of class names. Is it right to create a new property vs. adding to the existing? |
|
Another question: |
0.7.3 is what the unit tests and what my manual testing have used. I added this to the README. |
Both work, but adding it to the existing is probably the better approach. I updated the README to reflect this. |
I have never seen this. I need more context unfortunately.
|
|
That does not (should not) happen. It should only be there when after you have created it in the UI. Are you sure you haven't tried this a few times and either... When I was playing with this a few weeks ago and was seeing weird behavior, I found that (3) was a problem for me. I either left the Zeppelin process running accidentally or the stop script in Zeppelin does not work all the time. I had to |
|
I think creating the /interpreters/stellar/x.json creates the entry? I just tried again, ( new mvn install , new site file ) and I checked the interpreter log: INFO [2018-01-09 16:14:22,873] ({Thread-0} RemoteInterpreterServer.java[run]:97) - Starting remote interpreter server on port 63321 |
|
Can you run the check style against the interpreter class? |
Done. |
|
@nickwallen I'm not sure what to do to get this going. The jar is in maven. I tried to path directly to it as well. I can attach my site and interpreter json files, and screenshot my settings.... |
|
I'm going to try a clean zeppelin install I guess. |
|
ok, starting over
So I update and restart with new settings. Interpreter is green.
|
|
Ok, so it looks like it is working for you. Zeppelin interprets the first line as the interpreter spec (or whatever they call it). That's why |
|
Is the plan to deploy the jar file in the interpreter directory like the other interpreters? |
I don't know what that is going to look like. We need to do more research to see how the setup can be automated by the Mpack. I don't want the user to have to do all this manual setup. |
|
Do we have jiras for the next steps on this yet? I would be good if we recored our intent, and who knows, maybe someone will want to work on it. |
|
This is really good work @nickwallen. I am +1 on this, but would like to see jiras on the next steps entered if possible as a side note. |
|
Thanks for the reviews @ottobackwards and @merrimanr. Here are next steps for where I'd like to use this functionality. |



Stellar in a Zeppelin Notebook
The goal of this effort was to have the shell-like experience of the command-line driven REPL, but inside a web-based Zeppelin Notebook. This makes the existing functionality provided by the REPL more accessible and approachable for the average user.
This also opens up interesting additional scenarios like "executable" use cases or plotting profile data. Definitely more work to do on those fronts, but this is a first step.
Changes
There was a good amount of refactoring that I needed to do to achieve this. Prior to this effort different aspects of the REPL's functionality were tied together in the
StellarShellclass. To be able to have the same shell-like experience in both the REPL and Zeppelin, I needed to factor this logic out separately to allow for reuse.This included the following bits.
quitWe had very few automated tests around the shell/REPL, primarily because all these bits were tied together, which made testing difficult. Separating out this logic allowed me to add a large number of unit tests around each of these pieces of logic.
This also adds a separate project
metron-stellar/stellar-zeppelinthat provides a Zeppelin interpreter that allow you to run Stellar in a notebook. The interpreter acts as the front-end that then re-uses all of the bits that I refactored as described above.What does this do?
This allows you to run Stellar expressions in a Zeppelin Notebook.
This provides auto-complete in the Zeppelin Notebook by pressing the CTRL + . keys.
What does this not do?
This does not automatically prepare the Zeppelin environment to run Stellar. Meaning it does not install the Stellar interpreter in Zeppelin. You have to manually install the zinterpreter in Zeppelin by following the directions described in the
metron-stellar/stellar-zepelinREADME.Testing
Run Stellar in Notebook
metron-stellar/stellar-zepelin/README.mdto download/run Zeppelin locally.Validate the CLI REPL
Pull Request Checklist