neo4j-contrib / neomodel

An Object Graph Mapper (OGM) for the Neo4j graph database.
https://neomodel.readthedocs.io
MIT License
939 stars 231 forks source link

Updated ``neomodel.db.set_connection`` #730

Closed aanastasiou closed 1 year ago

aanastasiou commented 1 year ago

At its current state, neomodel.db.set_connection attempts to populate db.database_version by making a call to the database server. This assumes that at the time of calling set_connection the database server is already up and running.

This is not always the case and so far, the purpose of set_connection has been just that, to just set the connection, not to try and perform any operations on the database.

In any case, as useful as database_version is, it is not required all the time. Code that needs database_version will call it explicitly.

This PR attempts to fix this in the most straightforward way possible: database_version gets updated when required and that information is cached for future calls.

Another possibility is to automatically try to update database_version upon ensure_connection but at the moment I went for the most straightfowrard option.

mariusconjeaud commented 1 year ago

The database version is only needed to choose between the id() and elementId() Cypher methods indeed, so I think this is a good catch.

I resolved the merge conflicts and we can merge this in for release 5.1.1 then.

Thanks !

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information