-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HIVE-23715: Fix zookeeper ssl keystore password handling issues #1141
Conversation
+ " or " | ||
+ MetastoreDelegationTokenManager.DELEGATION_TOKEN_STORE_ZK_CONNECT_STR_ALTERNATE | ||
+ WHEN_ZK_DSTORE_MSG); | ||
throw new IllegalArgumentException("Zookeeper connect string has to be specified through " + "either " |
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.
Could you please remove extra concatenation of strings?
zkConnectPort = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.THRIFT_ZOOKEEPER_CLIENT_PORT); | ||
connectTimeoutMillis = (int) MetastoreConf | ||
.getTimeVar(conf, MetastoreConf.ConfVars.THRIFT_ZOOKEEPER_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); | ||
sslEnabled = MetastoreConf.getBoolVar(conf, MetastoreConf.ConfVars.THRIFT_ZOOKEEPER_SSL_ENABLE); |
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.
This is a bit awkward: if (sslEnabled) {} else {sslEnabled=..., if (sslEnabled)...}
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.
That else is for the previous if statement.
if (we use the alternate config)
read the alternate sslEnabled
if( that property is set)
read the other properties from the alternate config
else // we use the delegation own config
read the delagation sslEnabled config
if (the that property is set)
read the other properties from the delegation config
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.
Thanks!
close and reopen to rerun travis build |
…g issues (Peter Varga reviewed by Peter Vary) Closes apache#1141 (cherry picked from commit 439956d) Change-Id: I584b7344649e7d8eeec2aa2a29777ba6a2e7cfcc
HIVE-23715: Fix zookeeper ssl keystore password handling issues