neo4j / graph-data-science

Source code for the Neo4j Graph Data Science library of graph algorithms.
https://neo4j.com/docs/graph-data-science/current/
Other
621 stars 160 forks source link

feat: add progress tacker logger to pregel context #182

Closed frank-zsy closed 2 years ago

frank-zsy commented 2 years ago

Signed-off-by: Frankzhaopku syzhao1988@126.com

Before submitting this PR, please make sure:

This PR tends to add a build-in logger to Pregel framework. close #180

I looked into the code and InitContext and ComputeContext derive from NodeCentricContext which derives from PregelContext. And MasterComputeContext derives from PregelContext, so I add a private ProgressTracker reference into the PregelContext and expose three public functions logDebug, logWarning and logMessage which will call the same functions in the tracker, so developers can use them in all three contexts in the framework.

What I am not sure is:

frank-zsy commented 2 years ago

@s1ck could you have a look on this pull please?

s1ck commented 2 years ago

@frank-zsy Thanks for the PR. I was afk last week and didn't see it until now! Will have a look.

frank-zsy commented 2 years ago

Thanks, should I add the Logging section to 2.4 just before Message reducer or 2.6?

s1ck commented 2 years ago

Thanks, should I add the Logging section to 2.4 just before Message reducer or 2.6?

Let's do 2.6 Logging and briefly describe that the "following methods" are available at the three context objects and that they allow us to inject custom messages into the progress log of the algorithm execution.

frank-zsy commented 2 years ago

Thanks for the review, I looked into the documentation carefully and found almost equal number of function and method, is there any convention when to use which?

s1ck commented 2 years ago

Thanks for the review, I looked into the documentation carefully and found almost equal number of function and method, is there any convention when to use which?

It's true that it's mixed up, but since we use Java code snippets here, using method seems appropriate.

frank-zsy commented 2 years ago

Sure, thanks.