Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Incorporating Phil's changes to get SSL working again. #1

Merged
merged 2 commits into from Feb 25, 2016

Conversation

absurdfarce
Copy link
Owner

This commit represents a (slightly modified) version of commits from e3adcd0ff45ed029ffd7d68a2152e587e3b46a89
at https://github.com/philip-doctor/cassaforte. The only change from those commits is that the project owner
here is absurdfarce... pretty sure that's necessary to keep clojars happy.

This commit represents a (slightly modified) version of commits from e3adcd0ff45ed029ffd7d68a2152e587e3b46a89
at https://github.com/philip-doctor/cassaforte.  The only change from those commits is that the project owner
here is absurdfarce... pretty sure that's necessary to keep clojars happy.
@absurdfarce
Copy link
Owner Author

@philip-doctor I think I got everything from your branch... please double-check me and make sure I didn't miss anything.

(-> (JdkSSLOptions/builder)
(.withSSLContext ssl-context)
(.withCipherSuites ssl-cipher-suites))))
(.. (JdkSSLOptions$Builder.)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I get (and prefer) the .. syntax here... but why the change to instantiating a Builder class directly? JdkSSLOptions/builder does the same thing, doesn't it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with going back to (JdkSSLOptions/builder) it's probably a better choice in case DS java team starts adding logic to that method anyhow, so +1, the change I was debugging here is that we needed to (build) as a part of this and we were missing that step.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the missing build() call was... horrible. I have no idea how I missed that one... thanks for catching it!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem, I'm here to try to fix a bug, not pass judgement, don't worry about it :)

@absurdfarce
Copy link
Owner Author

@philip-doctor Apologies, the day got away from me a bit today. I think I covered everything we wanted to change here... can you take one more pass and make sure you're happy with what's here? If you are then I'll push up a new JAR containing these changes.

absurdfarce added a commit that referenced this pull request Feb 25, 2016
Incorporating Phil's changes to get SSL working again.
@absurdfarce absurdfarce merged commit ca376e5 into datastax-cass-30-drivers-cassaforte-201 Feb 25, 2016
@absurdfarce absurdfarce deleted the ssl-brokenness branch February 25, 2016 22:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants