[ZEPPELIN-2761] - Neo4j Interpreter #2478

conker84 · 2017-07-10T21:45:02Z

What is this PR for?

This contribution would to introduce Neo4j Cypher intepreter and at the same time provides base APIs that enable other graph databases (or graph framworks such as GraphX or Giraph).

What type of PR is it?

[Feature]

Todos

- Provide base APIs to manage graph results (under: org.apache.zeppelin.interpreter.graph.*)
- Create the Neo4j intepreter

What is the Jira issue?

[ZEPPELIN-2761]

How should this be tested?

Donwload and execute Neo4j v3.x, you can also pull a Docker image.

In order to execute test cases, if you are running Java 7, you need to also provide an environment variable telling the tests where to find Java 8, because Neo4j-the-database needs it to run.

export NEO4J_JAVA=<path/to/java/home>

Use this statement to create a dummy dataset

%neo4j
UNWIND range(1,100) as id
CREATE (p:Person {id:id, name: "Name " + id, age: id % 3}) WITH collect(p) as people
UNWIND people as p1
UNWIND range(1,10) as friend
WITH p1, people[(p1.id + friend) % size(people)] as p2
CREATE (p1)-[:KNOWS {years: abs(p2.id - p2.id)}]->(p2)

Then you can write some simple queries like:

%neo4j
MATCH (p:Person)-[r:KNOWS]-(p1:Person)
RETURN p, r, p1
LIMIT 10;

%neo4j
MATCH (p:Person)-[r:KNOWS]-(p1:Person)
RETURN p.id AS ID_A, p.name AS NAME_A, r.years AS YEARS, p1.id AS ID_B, p1.name AS NAME_B
LIMIT 20;

Video

Questions:

Does the licenses files need update? Yes
Is there breaking changes for older versions? No
Does this needs documentation? Yes

jexp · 2017-07-11T19:37:18Z

I looked through the code and the UX and can confirm that this is great work from a Neo4j point of view.

1ambda · 2017-07-17T05:29:22Z

Thanks for the great feature! since this is a big change, I guess it takes a time to review and requires multiple reviewers. Let me take a look.

1ambda · 2017-07-26T04:07:48Z

docs/index.md

  * [Lens](./interpreter/lens.html)
  * [Livy](./interpreter/livy.html)
  * [markdown](./interpreter/markdown.html)
+  * [Neo4j](./interpreter/neo4j.html)


We need to add the index into https://github.com/apache/zeppelin/blob/master/docs/_includes/themes/zeppelin/_navigation.html#L138 as well

Otherwise, it will not appear in navbar

1ambda · 2017-07-26T04:09:36Z

zeppelin-web/karma.conf.js

      'bower_components/ace-builds/src-noconflict/mode-python.js',
      'bower_components/ace-builds/src-noconflict/mode-sql.js',
      'bower_components/ace-builds/src-noconflict/mode-markdown.js',
-      'bower_components/ace-builds/src-noconflict/mode-pig.js',


Any reason to remove mode-pig.js?

1ambda · 2017-08-08T02:22:19Z

This is a new feature that makes utilizing graph type easily. Any other opinions?

jexp · 2017-08-08T11:04:10Z

@1ambda any update on the review progress?
We really would love to see this feature in Apache Zeppelin :)

Thanks a lot.

felixcheung · 2017-08-09T06:57:58Z

docs/interpreter/neo4j.md

+<div id="toc"></div>
+
+## Overview
+[Neo4j](https://neo4j.com/product/) is a native graph database, designed to store and process graphs from bottom to top. On the other hand, non-native solutions only add a shallow graph processing layer to an RDBMS or other NoSQL data stores, resulting in sub-optimal performance.


this part On the other hand, non-native solutions only add a shallow graph processing layer to an RDBMS or other NoSQL data stores, resulting in sub-optimal performance. sounds more like marketing :) think we can drop this from our doc

felixcheung · 2017-08-09T07:02:44Z

neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jConnectionManager.java

+        String username = properties.getProperty(NEO4J_AUTH_USER);
+        String password = properties.getProperty(NEO4J_AUTH_PASSWORD);
+        LOGGER.debug("Creating a BASIC authentication to neo4j with user '{}' and password '{}'",
+                username, password);


probably shouldn't log password?

You are totally right, it is my forgetfulness.
Removed

felixcheung · 2017-08-09T07:04:56Z

zeppelin-web/src/index.html

    <script src="bower_components/ace-builds/src-noconflict/mode-python.js"></script>
    <script src="bower_components/ace-builds/src-noconflict/mode-sql.js"></script>
    <script src="bower_components/ace-builds/src-noconflict/mode-markdown.js"></script>
-    <script src="bower_components/ace-builds/src-noconflict/mode-pig.js"></script>


why is this removed?

felixcheung · 2017-08-09T07:06:07Z

neo4j/src/main/resources/interpreter-setting.json

+      "neo4j.auth.password": {
+        "envName": null,
+        "propertyName": "neo4j.auth.password",
+        "defaultValue": "neo4j",


hmm, would it be better if the default value for password is ""?

felixcheung · 2017-08-09T07:07:09Z

neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java

+      }
+    } else if (obj instanceof Map) {
+      @SuppressWarnings("unchecked")
+      Map<String, Object> map = (Map<String, Object>) obj;


why do we need unchecked here?

felixcheung · 2017-08-09T07:08:04Z

neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java

+  public InterpreterResult interpret(String cypherQuery, InterpreterContext interpreterContext) {
+    logger.info("Opening session");
+    if (StringUtils.isEmpty(cypherQuery)) {
+      return new InterpreterResult(Code.ERROR, "Cypher query is Empty");


could you check other interpeters? it might be preferred to do nothing if the query is empty, instead of returning an error

felixcheung · 2017-08-09T07:08:30Z

neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jCypherInterpreter.java

+  @Override
+  public InterpreterResult interpret(String cypherQuery, InterpreterContext interpreterContext) {
+    logger.info("Opening session");
+    if (StringUtils.isEmpty(cypherQuery)) {


check isBlank instead

felixcheung · 2017-08-09T07:09:54Z

zeppelin-distribution/src/bin_license/LICENSE

    (Apache 2.0) frontend-maven-plugin 1.3 (com.github.eirslett:frontend-maven-plugin:1.3 - https://github.com/eirslett/frontend-maven-plugin/blob/frontend-plugins-1.3/LICENSE
    (Apache 2.0) frontend-plugin-core 1.3 (com.github.eirslett:frontend-plugin-core) - https://github.com/eirslett/frontend-maven-plugin/blob/frontend-plugins-1.3/LICENSE
    (Apache 2.0) mongo-java-driver 3.4.1 (org.mongodb:mongo-java-driver:3.4.1) - https://github.com/mongodb/mongo-java-driver/blob/master/LICENSE.txt
+    (Apache 2.0) Neo4j Java Driver (https://github.com/neo4j/neo4j-java-driver) - https://github.com/neo4j/neo4j-java-driver/blob/1.1/LICENSE.txt


I think we need to include neo4j-harness here

Hi @felixcheung ,
in the #1582 Moon said that "...org.neo4j.test:neo4j-harness is 'test' scope and will not bundled into the binary package, we don't need include it in zeppelin-distribution/src/bin_license/LICENSE file..."

Are you sure?

felixcheung · 2017-08-09T07:10:54Z

zeppelin-distribution/src/bin_license/LICENSE

    (Apache 2.0) frontend-maven-plugin 1.3 (com.github.eirslett:frontend-maven-plugin:1.3 - https://github.com/eirslett/frontend-maven-plugin/blob/frontend-plugins-1.3/LICENSE
    (Apache 2.0) frontend-plugin-core 1.3 (com.github.eirslett:frontend-plugin-core) - https://github.com/eirslett/frontend-maven-plugin/blob/frontend-plugins-1.3/LICENSE
    (Apache 2.0) mongo-java-driver 3.4.1 (org.mongodb:mongo-java-driver:3.4.1) - https://github.com/mongodb/mongo-java-driver/blob/master/LICENSE.txt
+    (Apache 2.0) Neo4j Java Driver (https://github.com/neo4j/neo4j-java-driver) - https://github.com/neo4j/neo4j-java-driver/blob/1.1/LICENSE.txt


we have version 1.4.0 in the pom file - should we have the same version for license instead of 1.1?

jexp · 2017-08-19T13:30:09Z

@conker84 ping :)

conker84 · 2017-08-19T13:37:10Z

@felixcheung let me know if i have to do something else!

conker84 · 2017-09-13T10:11:31Z

Hi guys, any news/plan about this?

felixcheung · 2017-09-13T16:11:26Z

sorry for dropping this. will review shortly.

jexp · 2017-09-14T05:00:56Z

@felixcheung thank you. We've also received requests from others waiting for this PR :)

felixcheung

ok LGTM with minor suggestions:

in the doc example/images/video we should avoid referencing political figures :)
could you please update them?
(optional) shall we move neo4j-java-driver to 1.4.3, and neo4j 3.2.3?

felixcheung · 2017-09-18T00:10:30Z

neo4j/src/main/resources/interpreter-setting.json

+      }
+    },
+    "editor": {
+      "language": "cypher",


note: there is no syntax highlighting support for cypher, so this won't do anything...

conker84 · 2017-09-24T12:45:01Z

Sorry for the late response, i lost it :(
@jexp what do you think about this
(optional) shall we move neo4j-java-driver to 1.4.3, and neo4j 3.2.3?
@felixcheung i'm updating the images :)

jexp · 2017-09-24T20:45:51Z

Yes please update.

conker84 · 2017-09-27T06:57:11Z

@felixcheung travis is failing, have i to rebase?

felixcheung · 2017-09-28T06:23:56Z

Let's try rebasing

felixcheung · 2017-09-28T06:24:16Z

I've reviewed the changes

conker84 · 2017-10-02T15:23:42Z

@felixcheung the job #10.2 fails...
Can you help me?

jexp · 2017-10-04T13:45:05Z

@felixcheung ping :)

conker84 · 2017-10-08T12:40:19Z

Hi @felixcheung any news?
Thanks
Andrea

felixcheung · 2017-10-09T00:42:03Z

could you rebase to pick up the change, I think the test failure is fixed by PR # 2609

felixcheung · 2017-10-09T16:47:23Z

since this is a known failure that is already fixed, let's not restart the process.
merging if no more comment

jexp · 2017-10-09T17:25:19Z

yay!! thanks so much.

conker84 · 2017-10-10T15:04:10Z

Thanks!

felixcheung · 2017-10-11T06:44:46Z

merged, thanks!

jexp · 2017-10-19T11:19:44Z

@felixcheung just curious when is the next Zeppelin release planned ? :)
Can't wait to see this out in the wild.

felixcheung · 2017-10-19T17:21:29Z

Not sure :) You can ask on dev@zeppelin.apache.org<mailto:dev@zeppelin.apache.org>

conker84 mentioned this pull request Jul 10, 2017

[ZEPPELIN-1604] Add Neo4j interpreter and Network visualization #1582

Closed

3 tasks

1ambda reviewed Jul 26, 2017

View reviewed changes

felixcheung requested changes Aug 9, 2017

View reviewed changes

felixcheung reviewed Sep 18, 2017

View reviewed changes

conker84 added 8 commits October 2, 2017 08:56

First commit

35b4e29

Create connection manager class

8e4690e

Moved business logic to the connection manager

2f88e98

Final behavior after discussion

9eb568d

Added docs

b5c2c66

Review by 1ambda 26/07

aaf618e

Review by felixcheung (09/08)

1e3d611

review by felixcheung 18/09

5fb97ce

asfgit closed this in e0fafac Oct 11, 2017

[ZEPPELIN-2761] - Neo4j Interpreter #2478

[ZEPPELIN-2761] - Neo4j Interpreter #2478

Uh oh!

Conversation

conker84 commented Jul 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Video

Questions:

Uh oh!

jexp commented Jul 11, 2017

Uh oh!

1ambda commented Jul 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1ambda commented Aug 8, 2017

Uh oh!

jexp commented Aug 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixcheung Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jexp commented Aug 19, 2017

Uh oh!

conker84 commented Aug 19, 2017

Uh oh!

conker84 commented Sep 13, 2017

Uh oh!

felixcheung commented Sep 13, 2017

Uh oh!

jexp commented Sep 14, 2017

Uh oh!

felixcheung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

conker84 commented Sep 24, 2017

Uh oh!

jexp commented Sep 24, 2017

Uh oh!

conker84 commented Sep 27, 2017

Uh oh!

felixcheung commented Sep 28, 2017

Uh oh!

felixcheung commented Sep 28, 2017

Uh oh!

conker84 commented Oct 2, 2017

conker84 commented Jul 10, 2017 •

edited

Loading

felixcheung Aug 9, 2017 •

edited

Loading