[
https://issues.apache.org/jira/browse/HBASE-12161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14161230#comment-14161230
]
Srikanth Srungarapu commented on HBASE-12161:
---------------------------------------------
Thanks for the detailed feedback Dima :). My responses inline
bq. Your method declaration for getAccessControlServiceStub should be split
over two lines. Same for the AccessControlClient.grant in
#testAccessControlClientGrantOnNameSpace.
Addressed.
bq. The AccessControlClient#grant and #revoke methods have nearly identical
logic that might as well be factored into a common private method to avoid a
bit of code duplication.
{code}
+ } finally {
+ if (ht != null) {
+ ht.close();
+ }
{code}
Agreed that this part of the code is common to both methods. But, is it worth
trying to refactor to special method?
bq. Similarly (as you explained to me), your
#testAccessControlClientGrantOnNameSpace is very similar to the existing
#testNamespaceUserGrant. It might be worth factoring out the common bits and
only leaving the different grant methods used by each test.
I found it a bit hard to refactor the commonalities into a method as the two
statements AccessTestAction and verifyXXXXX method calls are divided by
AccessControlClient.grant and grantOnNamespace methods respectively in each
test. So, instead I combined #testAccessControlClientGrantOnNameSpace and
#testAccessControlClientRevokOnNameSpace to reduce code duplication.
bq. In #testAccessControlClientGrantOnNameSpace, you handle a Throwable by just
logging it which swallow any exception that might break things. Is this
intentional or might you want the test to fail at this point if such a general
exception is thrown?
Yeah, this is deliberate as primary intention is to test functional correctness
of the methods (inline with the other defined tests in this class).
> Add support for grant/revoke on namespaces in AccessControlClient
> -----------------------------------------------------------------
>
> Key: HBASE-12161
> URL: https://issues.apache.org/jira/browse/HBASE-12161
> Project: HBase
> Issue Type: Improvement
> Reporter: Srikanth Srungarapu
> Assignee: Srikanth Srungarapu
> Priority: Minor
> Attachments: HBASE-12161_0.98.patch, HBASE-12161_master.patch
>
>
> As per the description.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
