Conversation
lmeyerov
left a comment
lmeyerov
left a comment
There was a problem hiding this comment.
-
did you mean to rm
.gitignore? -
is this creating a
:Urlnode with all props? (see slack conv..)
| property_value = '''"''' + property_value.replace('"',r"\"") + '''"''' | ||
| elif not property_value: | ||
| property_value = "" | ||
| property_value = "''" |
There was a problem hiding this comment.
@bechbd not sure of best practices here wrt neo4j
There was a problem hiding this comment.
this is only if the property value is None
| if count_node > prev_count_node + 1000: | ||
| prev_count_node = count_node | ||
| logger.info("> {} nodes already imported".format(count_node)) | ||
| logger.info("> {} nodes already merged".format(count_node)) |
There was a problem hiding this comment.
non-blocker, but generally better to do
logger.info("blah %s blah", myvar) so when in production mode, string interpolation methods don't execute ( => faster), esp. good when in data loops like this
There was a problem hiding this comment.
do you want me to update it right now?
There was a problem hiding this comment.
Up to you. Fine merging as-is, I just like being helpful in code reviews as a mutual learning process :)
3 participants
The code is working e2e, the big questions is what is the best way to include the neo4j class created in this issue into the master one.
After that the URL parsing and enrichment could be done easily.