neo4j-ruby-driver icon indicating copy to clipboard operation
neo4j-ruby-driver copied to clipboard

Config constant inconsistency?

Open leehericks opened this issue 6 years ago • 4 comments

One quick one: I noticed TrustStrategy.trust_all_certificates and LoadBalancingStrategy::LEAST_CONNECTED seem to be inconsistent. Typo?

######################################
# Example 2.8. Trust
######################################

driver = Neo4j::Driver::GraphDatabase.driver(uri, Neo4j::Driver::AuthTokens.basic(user, password),
                                             trust_strategy: Neo4j::Driver::Config::TrustStrategy.trust_all_certificates)

######################################
# Example Load balancing strategy
######################################

driver = Neo4j::Driver::GraphDatabase.driver(uri, Neo4j::Driver::AuthTokens.basic(user, password),
                                             load_balancing_strategy: Neo4j::Driver::Config::LoadBalancingStrategy::LEAST_CONNECTED)

leehericks avatar Jan 10 '19 07:01 leehericks

To be honest this is inspired by Java. They have it the same way there. The reason is that there various TrustStrategies that have to be described with a class and the class constructed in some way. On the other hand, the LoadBalancingStrategy has only 2 simple options. Maybe it should better be load_balancing_strategy: :least_connected but not sure if that could become a limitation in the future.

klobuczek avatar Jan 13 '19 19:01 klobuczek

@klobuczek I think it's ruby-like to do as you suggested but of course requires documentation to know the symbols.

load_balancing_strategy: :least_connected load_balancing_strategy: :round_robin

What about this for trust strategy settings? trust_strategy: :all trust_strategy: { signed_by: cert_file }

leehericks avatar Jan 14 '19 10:01 leehericks

While the load_balancing_strategyis probably safe to convert to pure hash, TrustStrategy is a class in Java with some functionality behind it which I'm hesitant to already give up and model with nested hash. Maybe that will happen when implementing the MRI driver based on seabolt. I'll have to see the level of support for that over there. The goal is to keep identical API between jruby and seabolt based driver.

klobuczek avatar Jan 15 '19 17:01 klobuczek

Ok, well I just thought it seemed inconsistent to have different programming strategies for the two options, especially when I saw how they defined TRUST_ALL_CERTIFICATES in the driver documentation. C#, Javascript, and Python all use enum or constants. Looks like they actually don't accept any cert file.

leehericks avatar Jan 16 '19 05:01 leehericks