Add Spring Data Neo4j support #5458
Conversation
…mentation updates
…o list of boot config factories.
spring-projects-issues
added
the
status: waiting-for-triage
snicoll
changed the title
snicoll
changed the title
snicoll
added
type: enhancement
|
Can you clarify the TODO please? I am not sure I get it. Hopper RC1 is already out. |
|
@snicoll The TODO is done - there are no longer any references to any Neo4j snapshot repositories. |
|
Alright, I've started to review the changes and I have a few questions:
I will push a first polish later today. Please do not commit on this branch anymore as I've changed quite a lot of things already. |
|
|
Sorry, I don't think that really answers all my questions. You named the property
Yes but we could read that info and pass it along. it's a String after all... Having said that, I have no idea how I would do that (yet) but it feel inconsistent right now.
Via configuration, something like And if you need to go remote you'd do In the first case, we can auto-detect the embedded driver. In the second the regular http driver. And you could still perfectly force the driver via the
The test is a copy/paste of the mongo one so I don't think that's XA related. |
|
No problem changing We could auto-detect the default drivers via the uri protocol. The default driver mode is embedded in-memory if you don't specify anything at all. So maybe:
would suffice. Though, |
|
Thanks for the reply. Yes, it's a bit harder to get things right. I'll give it some more thoughts. |
I've worked on a proposal that defaults to the package of the |
| In your `application properties`, you can supply any domain packages to be scanned by the OGM at startup | ||
| as well as the lifetime of the OGM session that will be established for web clients. | ||
|
|
||
| By default your application will be configured to use an in-process embedded instance of Neo4j that will not persist any data when your application shuts down. You can also connect to a remote Neo4j server, or to an embedded instance that persists data between restarts of your application. |
There was a problem hiding this comment.
Is that true? The starter does not bring the embedded driver so you need to add it yourself. What is the plan here? Should we add the embedded driver ourself? I think we should and that would be consistent with other areas where we support embedding.
In this case, the documentation is wrong. Again, no need to act on the code but please explain the intend.
ping @jexp @vince-bickers
|
A fix for DATAGRAPH-843 could improve our programming model though I managed to work-around it with a fake |
|
Team, I need some feedback. If I add the Defaulting on Have you tried to run that PR with the embedded driver at all? How can I fix this issue? |
|
looks like we can't use lucene 4 with Neo4j though this is the one we're using indirectly from other data project. |
That's a major problem for the Platform and, I would have thought, for the Spring Data release train as well. The boms become pointless if the things they contain don't work together. |
|
I agree this is a major blocker and if this is confirmed, I wish I'd know before spending so much time on this. |
This commit polihes the original Neo4j contribution in several areas. Rather than providing the packages to scan, this commit rearranges the `EntityScan` and `EntityScanRegistrar` so that the logic can be shared for other components. If no package is provided, scanning now defaults to the "auto-configured" package(s) and a `@NodeEntityScan` annotation allows to override that. The configuration has also been updated to detect the driver based on the `uri` property. If the embedded driver is available we use that by default. If it is not available, we're trying to contact a Neo4j server running on localhost. It is possible to disable the embedded mode or set the `uri` parameter explicitly to deviate from these defaults. Closes spring-projectsgh-5458
|
The sample starter project uses the embedded driver. It declares the following dependency:
|
|
If you run the sample it blows up since the test scope is not available. In any case, I don't think that's very relevant. We need to test the embedded driver and the sample is not the place for it. The real question is why you require Lucene 3.6? It's quite old. @vince-bickers is there a way I can get answer to the questions above? |
|
@snicoll Hi, sorry for the confusion. Regarding the main issue which is the Lucene dependency. After consulting with @olivergierke we would like to do it similarly as SD JPA does it with Hibernate. But in boot we would override it with a Neo4j 3.0 dependency which then pulls in Lucene 5.5. For your other comment: "We need to test the embedded driver and the sample is not the place for it." Can you point me to another project that uses their embedded/in-process test-drivers in the autoconfigure - tests? So that we can learn from it and update the PR (after pulling your changes? or what is your preferred process for us updating this PR?). Thanks a lot, Michael |
|
The code is (was) ready to be merged. Sorry, I don't know neo4j so explaining that the embedded driver is GPL would have helped. I now need to rework it to exclude the dependency on the embedded driver. Sample included which means we'll ressort to what we do for Redis. You should be able to run the sample, adding a test scope is not an option. If you want users to use Neo4j 3.0, why haven't you done that in your original submission? In other words, is it something you do because you have to or is that the target for your users base? |
|
I thought the licensing discussion of the test dependency (embedded driver) was sorted out on the email thread. That's why the embedded driver is only a test dependency in this PR. I'm not sure what you mean by:
Neo4j 3.0 is to be GA by April 26. Currently SDN was developed mainly against Neo4j 2.3 as default dependency but the underlying integration also works with Neo4j 3.0. We didn't default to 3.0 because SD-Hopper GA is due before Neo4j 3.0 will be GA. But as Boot 1.4 GA is scheduled for end of May, Neo4j will be final by then. (I had not checked the boot release plan). The main target for SDN 4.1 is Neo4j 2.3 (Server) but it will also work with Neo4j 3.0 for users that want to use it. |
That doesn't sound like a good solution to me. It means that users of Spring Data Hopper's bom who want to use Spring Data Elasticsearch and Spring Data Neo4J at the same time will not be able to do so with the out-of-the-box configuration. What's the point of a release train if its contents don't work together? |
|
They will. If they use Neo4j server, which users actually use usually. The embedded mode is really just a crutch to avoid the dependency to a real server. So this has been traded here in favor of adding yet another infrastructure component to the Boot build. But it's not the default way you'd use SDN 4. |
|
I confirm that all the compile and runtime dependencies of
are all Apache License v 2.0 (or compatible) licensed. |
* pr/5458: Polish contribution Add Neo4j support
No description provided.