[GitHub] mikewalch commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
mikewalch commented on a change in pull request #419: #408 - Removed
ClientConfiguration from ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r181157296
##
File path:
server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java
##
@@ -34,6 +32,10 @@ public Instance getInstance() {
if (instance == null) {
return HdfsZooInstance.getInstance();
}
-return new ZooKeeperInstance(this.getClientConfiguration());
+try {
+ return getConnector().getInstance();
+} catch (Exception e) {
+ throw new RuntimeException(e);
Review comment:
Fixed now
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
mikewalch commented on a change in pull request #419: #408 - Removed
ClientConfiguration from ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r181157214
##
File path:
server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java
##
@@ -36,6 +35,10 @@ synchronized public Instance getInstance() {
if (instance == null) {
return cachedInstance = HdfsZooInstance.getInstance();
}
-return cachedInstance = new ZooKeeperInstance(getClientConfiguration());
+try {
+ return cachedInstance = getConnector().getInstance();
+} catch (Exception e) {
+ throw new RuntimeException(e);
Review comment:
Should be fixed now
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
mikewalch commented on a change in pull request #419: #408 - Removed
ClientConfiguration from ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r181140657
##
File path: core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
##
@@ -282,15 +211,24 @@ public void parseArgs(String programName, String[] args,
Object... others) {
updateKerberosCredentials(clientConfigFile);
}
+ private ConnectionInfo cachedInfo = null;
+ private Connector cachedConnector = null;
protected Instance cachedInstance = null;
- protected ClientConfiguration cachedClientConfig = null;
+ private Properties cachedProps = null;
synchronized public Instance getInstance() {
-if (cachedInstance != null)
- return cachedInstance;
-if (mock)
- return cachedInstance = DeprecationUtil.makeMockInstance(instance);
-return cachedInstance = new
ZooKeeperInstance(this.getClientConfiguration());
+if (cachedInstance == null) {
+ if (mock) {
+cachedInstance = DeprecationUtil.makeMockInstance(instance);
Review comment:
I will remove it
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
mikewalch commented on a change in pull request #419: #408 - Removed
ClientConfiguration from ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r181133981
##
File path: core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
##
@@ -252,8 +182,7 @@ public void updateKerberosCredentials() {
* overwrite the options if the user has provided something specifically.
*/
public void updateKerberosCredentials(boolean clientSaslEnabled) {
-if ((saslEnabled || clientSaslEnabled) && null == tokenClassName) {
- tokenClassName = KerberosToken.CLASS_NAME;
+if (saslEnabled || clientSaslEnabled) {
Review comment:
I don't think it's needed anymore. I am going to push a commit that removes
it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
mikewalch commented on a change in pull request #419: #408 - Removed
ClientConfiguration from ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r181091502
##
File path: core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
##
@@ -195,19 +129,15 @@ public AuthenticationToken getToken() {
@Parameter(names = {"-fake", "--mock"}, description = "Use a mock Instance")
public boolean mock = false;
- @Parameter(names = "--site-file",
- description = "Read the given accumulo site file to find the accumulo
instance")
- public String siteFile = null;
-
@Parameter(names = "--ssl", description = "Connect to accumulo over SSL")
public boolean sslEnabled = false;
@Parameter(names = "--sasl", description = "Connecto to Accumulo using SASL
(supports Kerberos)")
public boolean saslEnabled = false;
@Parameter(names = "--config-file", description = "Read the given client
config file. "
- + "If omitted, the path searched can be specified with
$ACCUMULO_CLIENT_CONF_PATH, which "
- + "defaults to
~/.accumulo/config:$ACCUMULO_CONF_DIR/client.conf:/etc/accumulo/client.conf")
+ + "If omitted, the following paths will be searched
~/.accumulo/accumulo-client.properties:"
+ +
"$ACCUMULO_CONF_DIR/accumulo-client.properties:/etc/accumulo/accumulo-client.properties")
Review comment:
In general, I like having accumulo in the name as the file can be used in a
user's project and its clear what it is for. I like properties in the name as
it's more descriptive but I would be OK with `accumulo-client.conf`. However,
this should another issue as more code (outside of this PR) needs to changed if
we start using another name.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
mikewalch commented on a change in pull request #419: #408 - Removed
ClientConfiguration from ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r181089828
##
File path: assemble/bin/accumulo-cluster
##
@@ -244,7 +244,7 @@ function kill_all() {
done
echo "Cleaning all server entries in ZooKeeper"
- ${accumulo_cmd} org.apache.accumulo.server.util.ZooZap -master -tservers
-tracers --site-file "${conf}/accumulo-site.xml"
+ ${accumulo_cmd} org.apache.accumulo.server.util.ZooZap -master -tservers
-tracers
Review comment:
`ZooZap` doesn't use `ClientOpts` anymore which required the site file path
to be passed in. It still uses the site file as
`SiteConfiguration.getInstance()` is called.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
