----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63668/#review191000 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java Lines 113 (patched) <https://reviews.apache.org/r/63668/#comment268587> What if this returns an empty set? Should we return an empty list to skip the rest of the function body? If so, then perhaps returning a Collections.EMPTY_LIST would work? Btw, what about NULL values? could listAllRoles() return null in any case? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java Lines 109 (patched) <https://reviews.apache.org/r/63668/#comment268589> Same comment from the ShellGenericCommand. What about null values? is that possible? What if we have empty roles? should we return an empty list and skip the rest of the function body? - Sergio Pena On Nov. 8, 2017, 3:34 p.m., Colm O hEigeartaigh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63668/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2017, 3:34 p.m.) > > > Review request for sentry. > > > Bugs: SENTRY-2038 > https://issues.apache.org/jira/browse/SENTRY-2038 > > > Repository: sentry > > > Description > ------- > > This issue is for three fairly minor ShellCommand improvements: > > a) "roleName" is not required in ShellCommand.listRoles > b) Change the methods that split a "groups" String, to just take in a Set > instead. This means I can re-use them in the CLI branch. > c) Add a new "listGroupRoles" implementation, from the CLI branch. It's a > nice new command that allows you to list all groups and all roles associated > with those groups. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java > 49f18c89 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java > 11615ffa > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java > dd245eac > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java > 226d58d5 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java > ec751ecf > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java > 1e0692b5 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java > 80bbcf18 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java > 55831a45 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSqoop.java > 7bafd8c4 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java > adfd102c > > > Diff: https://reviews.apache.org/r/63668/diff/1/ > > > Testing > ------- > > Tested the script. > > > Thanks, > > Colm O hEigeartaigh > >
