-
Notifications
You must be signed in to change notification settings - Fork 105
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Pawel, this looks good overall. There's just some various small changes to make it better. Also can you run gofmt -w <filename>
on all files you edited, that will make sure indentation and such is correct. thanks!
cluster := gocql.NewCluster(strings.Split(addrs, ",")...) | ||
if ssl == true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can just put if ssl {
here
EnableHostVerification: false, | ||
} | ||
} | ||
if auth == true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if auth {
will do
keyspace string | ||
hosts string | ||
CaPath string | ||
Username string | ||
Password string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all these variables should be un-exported, i.e. start with a lower case letter.
(Enabled is an exception because it is read from the main package)
@@ -84,6 +94,19 @@ func New() *CasIdx { | |||
cluster.Timeout = timeout | |||
cluster.NumConns = numConns | |||
cluster.ProtoVersion = protoVer | |||
if Ssl == true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ssl {
will work
EnableHostVerification: false, | ||
} | ||
} | ||
if Auth == true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if auth {
if Ssl == true { | ||
cluster.SslOpts = &gocql.SslOptions { | ||
CaPath: CaPath, | ||
EnableHostVerification: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's turn this into a configurable option, hostVerification
because people will want to control this behavior (no need for "enable..." that's implied)
if ssl == true { | ||
cluster.SslOpts = &gocql.SslOptions { | ||
CaPath: CaPath, | ||
EnableHostVerification: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's turn this into a configurable option, hostVerification
because people will want to control this behavior (no need for "enable..." that's implied)
looks good, thanks! |
@Samaelson FYI i just pushed some cleanups, and some of the option names have changed a bit. now we also have adjusted config files in our packages and updated documentation |
Added a few new options such as authorization of user and ssl connection