-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-22271 Implement grant/revoke/delete table acls/delete namespace acls in Procedure #168
Conversation
private void updatePermissionStorage(MasterProcedureEnv env) throws IOException { | ||
try (Table table = | ||
env.getMasterServices().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { | ||
// update permission to acl table |
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.
whitespace:end of line
switch (state) { | ||
case UPDATE_PERMISSION_STORAGE: | ||
try { | ||
// update permission in acl table and acl znode |
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.
whitespace:end of line
} catch (KeeperException e) { | ||
LOG.error("Failed deleting acl node '{}'", zkNode, e); | ||
watcher.abort("Failed deleting node " + zkNode, e); | ||
} |
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.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
private void updatePermissionStorage(MasterProcedureEnv env) throws IOException { | ||
try (Table table = | ||
env.getMasterServices().getConnection().getTable(PermissionStorage.ACL_TABLE_NAME)) { | ||
// update permission to acl table |
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.
whitespace:end of line
switch (state) { | ||
case UPDATE_PERMISSION_STORAGE: | ||
try { | ||
// update permission in acl table and acl znode |
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.
whitespace:end of line
} catch (KeeperException e) { | ||
LOG.error("Failed deleting acl node '{}'", zkNode, e); | ||
watcher.abort("Failed deleting node " + zkNode, e); | ||
} |
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.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
PermissionStorage.addUserPermission(getConfiguration(), perm, table, | ||
mergeExistingPermissions); | ||
} | ||
ProcedurePrepareLatch latch = ProcedurePrepareLatch.createBlockingLatch(); |
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.
Maybe a better way is to add a proc id field in GrantResponse, and query the result at client side?
ProcedureExecutor<MasterProcedureEnv> masterProcedureExecutor = | ||
((HasMasterServices) c.getEnvironment()).getMasterServices() | ||
.getMasterProcedureExecutor(); | ||
ProcedurePrepareLatch latch = ProcedurePrepareLatch.createBlockingLatch(); |
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.
Do we really need to wait here?
ProcedureExecutor<MasterProcedureEnv> masterProcedureExecutor = | ||
((HasMasterServices) ctx.getEnvironment()).getMasterServices() | ||
.getMasterProcedureExecutor(); | ||
ProcedurePrepareLatch latch = ProcedurePrepareLatch.createBlockingLatch(); |
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.
Ditto.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@mymeiyi any update? Thanks. |
💔 -1 overall
This message was automatically generated. |
… acls in Procedure
💔 -1 overall
This message was automatically generated. |
Will close this PR in next week or so unless it comes back to life. Thanks. |
Close this PR and will split this task into several minor tasks. Thanks for all comments. |
Use UpdatePermissionProcedure to implement grant/revoke and RemovePermissionProcedure to implement delete namespace or table acls when delete namespace or table.