Does cartography actually need to explicitly call Neo4j consume()?

[achantavy]

@danielsaporo brought up that Neo4j client 5.x is required for use with Memgraph and calling consume() makes the transaction "out of scope" and throws an exception when you call it a second time. We call consume() multiple times in https://github.com/lyft/cartography/blob/48f50caed6ac889225e94611b1315f6a95c19470/cartography/graph/statement.py#L126 and in other places throughout the code.

The original intent of consume() was to fix #170. Now that write transactions (which include auto retries) are more prevalent in the code, I'm not sure if we still need consume().

We should validate whether this call to consume() is actually needed by removing it throughout the code and stress testing it with a real sync.

[danielsaporo]

@achantavy to make this very specific, the following changes would be necessary in statement.py:

• line 76: remove .consume()
• lines 120-128: remove entirely, except for line 121, which also needs to be unindented after the loop's removal

Obviously, the question is whether making these changes can cause any trouble.

From a quick search through the codebase, I do see other calls to consume() in other places, but can't say at the moment whether they cause any trouble with Neo4j client 5.x.

[achantavy]

lines 120-128: remove entirely, except for line 121, which also needs to be unindented after the loop's removal

I think we do need the call to consume in line 121 though because it maintains state. That loop is intended for cleanup jobs: it deletes LIMIT_SIZE objects from the graph one transaction at a time until the neo4j ResultSummary counters indicates that no further changes have happened in the graph. The loop does handle things one transaction at a time so I don't think it should cause any problems with Memgraph.

Other calls to consume are probably removable though - will experiment.