Fixes #558: Check for neo4j version being compatible with neo version #2700
Conversation
|
@vga91 please add a statement in the docs where you describe the behaviour of the log |
| @@ -5,6 +5,7 @@ | |||
| import org.neo4j.kernel.extension.ExtensionFactory; | |||
| import org.neo4j.kernel.extension.ExtensionType; | |||
| import org.neo4j.kernel.extension.context.ExtensionContext; | |||
| import org.neo4j.kernel.internal.Version; | |||
There was a problem hiding this comment.
this is internal class for a reason and should not be used here and there for any convience
There was a problem hiding this comment.
Oh I see. Therefore, please could you tell me what to use as an alternative to Version.getKernelVersion()?
There was a problem hiding this comment.
so there is a dbms.components procedure that will return neo4j version; also here you are expecting that version will also have dot separator and that will be major.minor - there is no guarantee that its the case
There was a problem hiding this comment.
so there is a dbms.components procedure that will return neo4j version; also here you are expecting that version will also have dot separator and that will be major.minor - there is no guarantee that its the case
conker84
added
4.4
core-functionality
| private String getMajorMinVersion(String completeVersion) { | ||
| if (completeVersion == null) { | ||
| return null; | ||
| } | ||
| final String[] split = completeVersion.split("\\."); | ||
| return split[0] + "." + split[1]; | ||
| } |
There was a problem hiding this comment.
Talked to @MishaDemianenko and there is no guarantee this is always on the major.minor format. Can we make it a bit more flexible and also add some sort of testing for this?
There was a problem hiding this comment.
I changed the handling. Now we check if <KERNEL_VERSION> starts with <APOC_VERSION>,
because expect that APOC versioning is always consistent to Neo4j versioning,
e.g. if we have Neo4j 5_0_0, the Apoc version will be 5_x_x_x as well.
And I added some tests too.
vga91
force-pushed
the
issue_558
branch
4 times, most recently
from
a91ab9c
to
774d6bb
Compare
| } | ||
| final String[] split = completeVersion.split("\\."); | ||
| return split[0] + "." + split[1]; | ||
| // only for testing purpose |
There was a problem hiding this comment.
Do you refer to that it is public for testing purposes and could be private otherwise? In that case, maybe the comment can explicitly say so.
There was a problem hiding this comment.
Yes exactly. Added it and fixed the failed test as well
There was a problem hiding this comment.
Looks good to me now, but would like @MishaDemianenko to have another look as well to see if he thinks his concerns are fixed
|
Thank you @vga91 and thank you @Lojjs and @MishaDemianenko for the review! |
…h neo version (neo4j-contrib#2700) * Fixes neo4j-contrib#558: Check for neo4j version being compatible with neo version * doc addition - changed neo4j version getting * added test - changed version handling * edit comment
…h neo version (neo4j-contrib#2700) * Fixes neo4j-contrib#558: Check for neo4j version being compatible with neo version * doc addition - changed neo4j version getting * added test - changed version handling * edit comment
…h neo version (neo4j-contrib#2700) * Fixes neo4j-contrib#558: Check for neo4j version being compatible with neo version * doc addition - changed neo4j version getting * added test - changed version handling * edit comment
…h neo version (neo4j-contrib#2700) * Fixes neo4j-contrib#558: Check for neo4j version being compatible with neo version * doc addition - changed neo4j version getting * added test - changed version handling * edit comment
…h neo version (neo4j-contrib#2700) * Fixes neo4j-contrib#558: Check for neo4j version being compatible with neo version * doc addition - changed neo4j version getting * added test - changed version handling * edit comment
Fixes #558
Added a check between neo4j and apoc version. If versions are different, a log warn will be returned.