search

neo4j-rstats / neo4r

A Modern and Flexible Neo4J Driver
https://neo4j-rstats.github.io/user-guide/
Other
106 stars 30 forks source link

Neo4j 4.x Support #79

Open toddgrabowsky opened 3 years ago

toddgrabowsky commented 3 years ago

Thanks for all the great work on neo4r! I made an attempt at updating the package to support Neo4j 4.x without impacting previous Neo4j versions.

From what I've read about the HTTP API for Neo4j 4.x, it appears that the result format is the same (so no updates to result parsing), but the following updates have been made:

The updates in this PR basically contain some conditional logic to accommodate the 4.x way of doing things and the <4.x way of doing things. Would love to hear your thoughts to see if this is a good path for making neo4r compatible with Neo4j 4.x.

davidlrosenblum commented 3 years ago

We have arrived at a similar place from two slightly different directions.

I bailed on calling the old endpoints and do all cypher calls, whether 3.x or 4.x I allowed the selection of another database besides neo4j for Neo4j Enterprise users. I assume less about the availability of the database and maybe interrogate thing a bit too much My self$ping() is a breaking change (true/false) instead of the 200 being good. This has proved to be a challenge for multi-database. My R coding is extremely rusty.

Line 171 I think you meant self, not con4 (yeah, I also made that mistake) call_neo4j(con4) %>%

You can see what I mean here: https://github.com/davidlrosenblum/neo4r/tree/4.x https://github.com/davidlrosenblum/neo4r/tree/4.x

We had a call with Colin about a month ago - and the results were much more around system db handling and figuring out the database version (which you do).

On Oct 20, 2020, at 3:52 PM, Todd Grabowsky notifications@github.com wrote:

Thanks for all the great work on neo4r! I made an attempt at updating the package to support Neo4j 4.x without impacting previous Neo4j versions.

From what I've read about the HTTP API for Neo4j 4.x https://neo4j.com/docs/http-api/current/, it appears that the result format is the same (so no updates to result parsing), but the following updates have been made:

the REST API endpoints have been deprecated (which means we need to write a cypher query to get constraints, indexes, labels, relationship_types, etc. for 4.x) the transaction endpoint has changed from /db/data/transaction/commit to /db/{database_name}/tx/commit the POST body for the request has a new "parameters" element https://neo4j.com/docs/http-api/current/actions/query-result/ The updates in this PR basically contain some conditional logic to accommodate the 4.x way of doing things and the <4.x way of doing things. Would love to hear your thoughts to see if this is a good path for making neo4r compatible with Neo4j 4.x.

You can view, comment on, or merge this pull request online at:

https://github.com/neo4j-rstats/neo4r/pull/79 https://github.com/neo4j-rstats/neo4r/pull/79 Commit Summary

Incremented version of package in the DESCRIPTION file. Updated R6 class definition to work with both Neo4j 3.x and 4.x. updated call_api function to work with both 3.x and 4.x. updated roxygen documentation for package using devtools::document(). use base R version of the substring function to avoid importing a function from stringr. add map_dfr function from the purrr package to the imports for the Neo connection R6 class. File Changes

M DESCRIPTION https://github.com/neo4j-rstats/neo4r/pull/79/files#diff-9cc358405149db607ff830a16f0b4b21f7366e3c99ec00d52800acebe21b231c (4) M NAMESPACE https://github.com/neo4j-rstats/neo4r/pull/79/files#diff-b6b6854a02ae177f8860f49654ff250c1554d021ae434b6efdbe99d00bdc6055 (1) M R/R6.R https://github.com/neo4j-rstats/neo4r/pull/79/files#diff-eab71897add494e0011233a65002c007c66c7bafb229d8ba184ee1dfc3ee3020 (92) M R/call_api.R https://github.com/neo4j-rstats/neo4r/pull/79/files#diff-2bf43af4cf7860ae6b89156d60569edb81bafbc711db96b2270998da5ba99266 (69) M man/call_neo4j.Rd https://github.com/neo4j-rstats/neo4r/pull/79/files#diff-27693fb6e6bd145db8cd282ac9c44b863017b44b3a203bd9091d7a1c3af643e5 (5) M man/neo4j_api.Rd https://github.com/neo4j-rstats/neo4r/pull/79/files#diff-6999e59b7f378dc734526310ce53a67cc10e64577031753b051e73802b2ebd7b (2) Patch Links:

https://github.com/neo4j-rstats/neo4r/pull/79.patch https://github.com/neo4j-rstats/neo4r/pull/79.patch https://github.com/neo4j-rstats/neo4r/pull/79.diff https://github.com/neo4j-rstats/neo4r/pull/79.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/neo4j-rstats/neo4r/pull/79, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFBXAGGHET77S5Z7JXBLZH3SLXTBVANCNFSM4SYZXJKQ.

toddgrabowsky commented 3 years ago

Thanks @davidlrosenblum ! Just added a new commit to fix the "con4" issue. Thanks for catching that.

To determine which version of Neo4j is being used, I'm basically looking at the structure of the results from a GET request to the root endpoint to try and determine 4.x or pre-4.x. Do you think that's a reasonable way to distinguish between 4.x and pre-4.x or is there a better way? Once we can infer the version of Neo4j, it's basically just some if-else logic to handle things pre vs. post 4.x.

toddgrabowsky commented 3 years ago

@davidlrosenblum I like that you made "db" a public member of the Neo4j connection object. That makes a lot of sense as opposed to specifying the "db" as part of the call_neo4j function. I'll continue taking a look at your updates and would love to hear any thoughts/feedback about the changes I proposed. I'm sure there are probably a lot of use cases related to Neo4j 4.x Enterprise that haven't considered.

davidlrosenblum commented 3 years ago

Hi Todd,

I work for Neo4j - and I have prospects and customers who want to use an R connector that supports 4.x. That was my driver for update this. We first had a very light touch version when is functionally equivalent to where you seem to be today, with the one difference that we supported selecting a database. Notable is that we took the exact same approach to flattening the output from call db.indexes by unwinding the output. The feedback for that was to do a bit more around Enterprise and 4.x functionality, auto detect version (I originally made the user pick, old or new, we elected to go the auto select route. I like how efficient your code is. I am not sure that I need to go thru the discovery process I went thru to determine version, etc. We (the folks at Neo4j) then spent some time adding the functionality we thought needed, that was still true to what Colin’s original spirit was. We are doing a code cleanup this week, looking to put in a PR at some point there. IDK what happens after that (not under my control).

On Oct 20, 2020, at 4:55 PM, Todd Grabowsky notifications@github.com wrote:

@davidlrosenblum https://github.com/davidlrosenblum I like that you made "db" a public member of the Neo4j connection object. That makes a lot of sense as opposed to specifying the "db" as part of the call_neo4j function. I'll continue taking a look at your updates and would love to hear any thoughts/feedback about the changes I proposed. I'm sure there are probably a lot of use cases related to Neo4j 4.x Enterprise that haven't considered.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/neo4j-rstats/neo4r/pull/79#issuecomment-713134003, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFBXAGCZJ43TN26QRRETW3LSLX2LNANCNFSM4SYZXJKQ.

ColinFay commented 3 years ago

Thanks @toddgrabowsky for the awesome work!

I've just discussed with David that pointed that he's going to reuse some part of your code.

We'd love to list you as a contributor to the package one this all thing is merged, are you ok with being listed in the DESCRIPTION ?

toddgrabowsky commented 3 years ago

@ColinFay that would be great! Thanks!

fbiville commented 3 years ago

Hi, is there any update on this pull request? Is it in a usable state?

funarog commented 3 years ago

Is there an available update to connect to neo4j version 4?

elliliao commented 3 years ago

May I ask if there are updates on this?