ctubbsii commented on code in PR #58:
URL: https://github.com/apache/accumulo-proxy/pull/58#discussion_r1048843081
##########
src/main/thrift/proxy.thrift:
##########
@@ -346,7 +337,6 @@ service AccumuloProxy {
)
void addSplits (
- 1:binary login
Review Comment:
I think you're probably going to need to keep the `login` field, but use it
differently. It is currently used to track authenticated sessions. I think we
still need that, so the preferred language client can authenticate to the proxy
with some kind of shared secret. The task at hand is to eliminate the need for
the preferred language client to instruct the proxy on how to authenticate to
Accumulo, because the proxy should be able to authenticate to Accumulo using
the client properties file. But, we still want the proxy to not just allow
anybody to connect to it and use it. So, the preferred language client should
have some way to authenticate to the proxy. For that, this thrift field can be
reused to pass that secret shared between the preferred language client and the
proxy.
##########
src/main/java/org/apache/accumulo/proxy/ProxyServer.java:
##########
@@ -188,10 +183,7 @@ public void
onRemoval(RemovalNotification<UUID,ConditionalWriter> notification)
public ProxyServer(Properties props) {
- @SuppressWarnings("deprecation")
- org.apache.accumulo.core.client.Instance i = new
org.apache.accumulo.core.client.ZooKeeperInstance(
- ClientConfConverter.toClientConf(props));
- instance = i;
+ client = Accumulo.newClient().from(props).build();
try {
String tokenProp = props.getProperty("tokenClass",
PasswordToken.class.getName());
Review Comment:
I don't think you need this check anymore. `Accumulo.newClient()` should be
able to set up the `client` using whatever token type it supports. That
restriction was in place before because the preferred language client could
only support passwords... but now that we're not logging in using the preferred
language client, and the proxy is setting up the client on its own, we don't
need to restrict it.
##########
src/main/java/org/apache/accumulo/proxy/ProxyServer.java:
##########
@@ -707,41 +682,41 @@ public void setLocalityGroups(ByteBuffer login, String
tableName,
groups.get(groupEntry.getKey()).add(new Text(val));
}
}
- getConnector(login).tableOperations().setLocalityGroups(tableName,
groups);
+ client.tableOperations().setLocalityGroups(tableName, groups);
} catch (Exception e) {
handleExceptionTNF(e);
}
}
@Override
- public void setTableProperty(ByteBuffer login, String tableName, String
property, String value)
Review Comment:
The shared secret can just be a String. We don't need it to be binary
anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
