search

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
639 stars 161 forks source link

OpenGDS compatibility disabled for neo4j 4.4.0 #146

Closed ppatino closed 2 years ago

ppatino commented 2 years ago

Describe the bug I have previously compiled, packaged, and deployed OpenGDS to a local neo4j instance, but ran into problems with doing this on v1.8.0 (I need compatibility to neo4j 4.4.0). I believe I tracked this down to this commit from 28 days ago, which explicitly disabled compatibility to neo4j 4.4.0: "Exclude 4.4 compat from openGDS. 4.4.0 doesn't exist yet.".

I initially thought it would be as easy as undoing this commit's exclusion, but after doing that & rebuilding/packaging, it appears I am still having the same issue.

To Reproduce GDS version: 1.8.0 Neo4j version: 4.4.0 Operating system: MacOS 12.0.1

Steps to reproduce the behavior:

Expected behavior An error occurs: Neo.ClientError.Procedure.ProcedureCallFailed Unable to inject component to field allocationTracker, please ensure it is public and non-final: Could not load the class org.neo4j.gds.compat.Neo4jProxy implementation for 4.4

ppatino commented 2 years ago

Actually... between this and a couple of other changes, it looks like I got it to work. In case it helps anyone else in the meantime, here are the changes I made, but I'm stopping short of trying to push a contribution so that someone who actually understands this project can make sure it's done correctly :).

image

Edit: accidentally clicked Close instead of comment.

Mats-SX commented 2 years ago

@ppatino Thank you for reaching out to us with this issue! We will make sure to fix it right away.

Mats-SX commented 2 years ago

@ppatino I've pushed your fix to 1.8: https://github.com/neo4j/graph-data-science/commit/88dd1afe084952642bf3b6d6bb87c28b7bf416ed

This seems to work. I had trouble locating your exact account details for the co-authorship, apologies for that.

I believe this resolves this issue.

ppatino commented 2 years ago

Excellent - thank you for doing that @Mats-SX! I should have pushed a commit :) Just wan't confident this was not going to affect anything else I was unaware of. And looks like I had missed the storage engine adapter 4.4 inclusion.