JAVA-3041 Guava session - cloud bundle support. replaced deprecated constructors#1606
Conversation
|
Hello, @smatvienko-tb. Mica brought this to our attention. Thanks for submitting a PR. And thanks for completing the contributor license agreement (CLA). Just wondering if you've logged a jira for this. I tried to do a search but couldn't find one. You can log a ticket here and link the PR. We're a little behind but I'll get someone to do a review and let you know. Cheers! 🍻 |
|
Hi, @ErickRamirezAU! Here is the new Jira ticket for this PR: https://datastax-oss.atlassian.net/browse/JAVA-3041 Stay tuned! |
Thanks! 👍 |
|
So what appears to be happening here is that the necessary config vals for Astra aren't passed through by the old constructor-based mechanism. SessionBuilder.buildContext() will prefer to use the constructor option if it returns a non-null value; this allows subclasses to define their own context impls. Problem is that the constructor option doesn't allow for every option to be passed; it looks to have been deprecated before some were added, most relevant to our case before the Astra config values were updated. And the calling code in this case builds up most of it's data in programmatic arguments before calling buildContext(). So if buildContext() doesn't have every value to be preserved passed as an argument it won't be able to create a ProgrammaticBuilder instance that includes them. Upshot: any values that aren't passed as args to buildContext() are basically left out. To prove this out try the following change: diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java b/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java
index 966372fa2..a6eaa0c99 100644
--- a/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java
+++ b/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java
@@ -60,6 +60,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
@@ -900,8 +901,10 @@ public abstract class SessionBuilder<SelfT extends SessionBuilder, SessionT> {
CqlIdentifier.fromCql(defaultConfig.getString(DefaultDriverOption.SESSION_KEYSPACE));
}
+ DriverContext ctx = buildContext(configLoader, programmaticArguments);
+ Optional<SslEngineFactory> sslFactory = ctx.getSslEngineFactory();
return DefaultSession.init(
- (InternalDriverContext) buildContext(configLoader, programmaticArguments),
+ (InternalDriverContext) ctx,
contactPoints,
keyspace);Take a look at sslFactory in a debugger. When using the current 4.14.1 impl this value is an empty option; the SSL factory configs aren't preserved in the args passed to GuavaSessionBuilder.buildSession(). Applying the changes in this PR you'll see that the sslFactory is now a non-empty value. This also explains the observed error; because the SSL configs aren't included Astra hangs up before we even get a chance to pass in an OPTIONS message. Note that it's the combination of the Guava session with Astra that brings out the problem here. The example in the docs should work fine if Astra isn't involved. Need to think a bit about the best way to address this problem. It's probably okay to make the change contained in this PR but at a minimum we'll also want to update the docs at the same time. |
|
The documentation seems to be fine. It does not expose the details and refers directly to the git repo. Probably updating the repo is just fine for this page: https://docs.datastax.com/en/developer/java-driver/4.4/manual/developer/request_execution/ Connecting to the Astra example is welcomed. Probably, the better compatibility with the install scripts for local Cassandra will be accepting "CREATE KEYSPACE IF NOT EXISTS" and report success if the keyspace already exists. |
|
Apologies @smatvienko-tb , I've been off working on other projects for the past little while. :( It's the docs you cite above that originally had me concerned. Note the ambiguity of the "buildContext(...)" call in the sample code you cited: public class GuavaSessionBuilder extends SessionBuilder<GuavaSessionBuilder, GuavaSession> {
@Override
protected DriverContext buildContext( ... ) { ... }
@Override
protected GuavaSession wrap(CqlSession defaultSession) {
return new DefaultGuavaSession(defaultSession);
}My concern was that this code wasn't clearly steering users towards the ProgrammaticArguments version introduced in JAVA-2315. The more I look at it there more I'm inclined to agree with you, though; I think the docs are probably fine. They point users at the buildContext() method generally, and from there it should be fairly clear which method should be overridden... especially if our sample code now uses the correct method (which it does with your changes in this PR). I may still rethink this and change the docs later to show an explicit override of buildContext(ProgrammaticArguments) but it's probably okay for now. |
|
Thanks for the contribution (and for the dialogue) @smatvienko-tb ! |



Guava session - cloud bundle support
Hey from ThingsBoard team!
I want to share my experience connecting Datastax Astra DB cloud with GuavaSession using Thingsboard.
The old GuavaSession implementation does not connect with cloud Astra DB.
This is because deprecated constructors were in place.
The paper that describes how to implement GuavaSession instead CqlSession is here:
https://docs.datastax.com/en/developer/java-driver/4.4/manual/developer/request_execution/
Here are the screenshots before I tried to connect with the old version:
Now it works like a charm.
Here is the code example of how I tested the Astra cloud connection with CqlSession and guavaSession