EdColeman commented on code in PR #3122:
URL: https://github.com/apache/accumulo/pull/3122#discussion_r1048990629
##########
server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java:
##########
@@ -106,7 +106,7 @@ public static PermissionHandler
getPermHandler(ServerContext context) {
protected SecurityOperation(ServerContext context, Authorizor author,
Authenticator authent,
PermissionHandler pm) {
this.context = context;
- zkUserPath = Constants.ZROOT + "/" + context.getInstanceID() + "/users";
+ zkUserPath = Constants.ZROOT + "/" + context.getInstanceID() +
Constants.ZUSERS;
Review Comment:
Maybe as a follow-on? If I recall, almost always these path fragments are
concatenated to build a path and the actual component name is not needed in
isolation. The issue is that the instance id is a runtime value - but we could
migrate to a set of static methods that accepted an instance id and returned a
fully formed path.
The start usually is built with `ZooUtil.getRoot(iid)` and then a bunch of
concatenations that may be easier to work with if it was along the lines of
`ZooUtil.getZkUserPath(iid, userName)`
It may be preferable to commit this as it maintains the current convention
and the use follow-on(s) to reduce the usage of Constant values everywhere.
Or, maybe memorize then on context?
Not sure which approach would be best, just providing a suggestion for
discussion.
--
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]
