Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Neo4j export #91
Neo4j export #91
Changes from 3 commits
b9f2081
8b6714c
c0d9558
80dc844
7abcbd7
234f057
9f7225d
e9cf1c5
faabd7e
6f0df20
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Mention that the value of this attribute can be a comma separated list of label values if user wants multiple labels. Would be more elegant to allow vectors here, but probably now is not the time.
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 expect you usually just use one label. Described the commas in the help.
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.
When I first saw this (and of course didn't realize I just reviewed the definition of this class :( ) I thought why would you put a connection kind of class up here, why not create the connection only when needed, in execute. But of course, there is no connection being made here, it just collects some metadata which will be used in a helper method of this object. So then what about naming it differently? E.g. Neo4JHelper?
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 changed it to Neo4jConnectionParameters.
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.
Does this make sense?
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.
Yeah, I think this is perfect! But what about adding GUID as vertex and edge attribute as well? That would allow users to clean up results of somehow screwed up exports with relative ease. Maybe we can add export date as well?
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.
Actually the timestamp is better than the GUID. Maybe you've recreated your LynxKite instance. Maybe two LynxKite instances use the same Neo4j instance. If everyone exports the example graph, you can end up with two exports with the same GUID quick enough.
I'll switch to just using the timestamp. Easier to interpret too.
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.
👍
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.
See https://neo4j.com/labs/apoc/4.1/overview/apoc.create/apoc.create.node/.
This is the way to create a node with a dynamically picked label. Requires
apoc
to be installed.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.
See https://neo4j.com/labs/apoc/4.1/overview/apoc.create/apoc.create.relationship/.
Same deal.