graph.close: only serialise to file if necessary #326
Conversation
i.e. if file doesn't exist yet or there have been changes to the graph
| val (nodes, edges, props) = Serialization.writeGraph(this, storagePath) | ||
| logger.debug(s"closed graph: wrote $nodes nodes with $edges edges and $props properties") | ||
| if (!Files.exists(storagePath) || hasChangedSinceOpen) { | ||
| logger.info(s"closing graph: writing to storage at `$storagePath`") |
There was a problem hiding this comment.
I think we might always want a log message that the graph was closed. Just with an indication if it is written or not.
| logger.info(s"closing graph: writing to storage at `$storagePath`") | ||
| val (nodes, edges, props) = Serialization.writeGraph(this, storagePath) | ||
| logger.debug(s"closed graph: wrote $nodes nodes with $edges edges and $props properties") | ||
| if (!Files.exists(storagePath) || hasChangedSinceOpen) { |
There was a problem hiding this comment.
Why do we want to write the cpg if the file does not exist and no changes have been applied?
I can only think of this happening if somebody deleted the cpg externally but then it is not our concern to write it out again.
| @@ -153,13 +153,15 @@ class Graph(val schema: Schema, val storagePathMaybe: Option[Path] = None) exten | |||
| def isClosed: Boolean = closed | |||
|
|
|||
| override def close(): Unit = { | |||
| logger.debug("closing") | |||
There was a problem hiding this comment.
Most people do not have their logger config set up to print the emitting class names. In those case it is not clear what it being closed. So something like "closing graph" would be much more helpful.
There was a problem hiding this comment.
"most people" don't make wise decisions :)
There was a problem hiding this comment.
adapted, will merge on green
| if hasChangedSinceOpen | ||
| } Serialization.writeGraph(this, storagePath) | ||
|
|
||
| logger.info("closed") |
i.e. if file doesn't exist yet or there have been changes to the graph