---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34086/#review128081
---
LGTM
- Jerry Chen
On April 11, 2016, 6:20 a.m., Colin Ma wrote
directly assigned to the user.
- Jerry Chen
On April 8, 2016, 6:40 a.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit
at
we pass requtorUserNames.
If we want to use new cases to cover this and don't want to modify here, I
suggest to still use null.
- Jerry Chen
On April 8, 2016, 4:26 a.m., Colin Ma wrote:
>
> ---
> This is an
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34079/#review127737
---
Ship it!
Ship It!
- Jerry Chen
On April 8, 2016, 3:13 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34082/#review127735
---
Ship it!
Ship It!
- Jerry Chen
On April 8, 2016, 3:45 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45730/#review127566
---
Ship it!
Ship It!
- Jerry Chen
On April 6, 2016, 3:56 a.m
r what this is for: show the reason how the users and groups is
related in testing roles for user feature?
- Jerry Chen
On April 6, 2016, 2:58 p.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mai
/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/34086/#comment190942>
Question: why we need to remove the exception handler?
- Jerry Chen
On April 6, 2016, 2:56 p.m., Colin Ma
R to construct the userNames.
requestorUserNames = Sets.newHashSet(ADMIN_USER);
And perform grantRoleToUser similar to group to test both groups and users
in this case instead of passing the empty user list.
- Jerry Chen
On April 6, 2016, 2:56 p.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34083/#review127553
---
LGTM
- Jerry Chen
On April 6, 2016, 2:55 p.m., Colin Ma wrote
icy/indexer/SimpleIndexerPolicyEngine.java
(line 95)
<https://reviews.apache.org/r/34082/#comment190912>
Debug log as:
("Getting permissions for groups: {}, users: {}", groups, users);
The same comment for several other instances.
- Jerry Chen
On April 6, 2016
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34081/#review127550
---
LGTM
- Jerry Chen
On April 6, 2016, 2:50 p.m., Colin Ma wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34080/#review127549
---
LGTM
- Jerry Chen
On April 6, 2016, 2:48 p.m., Colin Ma wrote
st/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
(line 2016)
<https://reviews.apache.org/r/34079/#comment190895>
Suggest to pass null instead of new HashSet(). The same for the
next instance.
- Jerry Chen
On
/sentry_policy_service.thrift
(line 109)
<https://reviews.apache.org/r/34078/#comment190889>
Please don't change the index of field if we don't see specific reasons
here.
- Jerry Chen
On April 6, 2016, 2:39 p.m., Colin Ma wrote:
>
>
/sentry_policy_service.thrift
(line 231)
<https://reviews.apache.org/r/34078/#comment190539>
required field of users will make the thrift API not compatible.
Suggest optional for this to keep API compatible.
- Jerry Chen
On April 6, 2016, 5:09 a.m., Colin Ma
me as null. but test it to make sure.
- Jerry Chen
On April 6, 2016, 2:52 a.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https:/
/sentry/provider/db/service/persistent/SentryStore.java
(line 2080)
<https://reviews.apache.org/r/45727/#comment190504>
Wrong indent spaces. 6 here.
There are a lot other instances with 8 spaces indent including the code in
tests. Should comply the format style.
- Jerry Chen
On
/sentry/service/thrift/SentryServiceUtil.java
(line 72)
<https://reviews.apache.org/r/45730/#comment190502>
This static method is not trictly related to export. It is an general
object path.
Should name as parseObjectPath.
- Jerry Chen
On April 6, 2016, 2:38 a.m., Colin Ma
lines. But
here we saw 8 spaces.
There are a lot of other instances. Please correct them too.
- Jerry Chen
On April 6, 2016, 2:38 a.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mail. To r
/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
(line 912)
<https://reviews.apache.org/r/45730/#comment190216>
Can we add a new method if we cannot found instead of using
convertToTSentryPrivilege which looks like a little wired?
- Jerry Chen
On April 5, 2016, 8:
ere is identical to the comments added
above.
- Jerry Chen
On April 5, 2016, 8:03 a.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mail. To reply, vi
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45727/#review127061
---
LGTM
- Jerry Chen
On April 5, 2016, 7:38 a.m., Colin Ma wrote
return some role set and this role set
will be passed to getGroupNameRoleNamesMap. This will not be the same as we
pass getGroupNameRoleNamesMap(null).
While we may want getGroupNameRoleNamesMap(null) for databaseName == null
and tableName == null case.
- Jerry Chen
On April 5, 2016, 7:
l help.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
(line 2113)
<https://reviews.apache.org/r/45727/#comment190200>
Please early return if null to keep the indent depth minimum.
- Jerry Chen
On April 5, 2016, 6:48 a.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45728/#review127053
---
LGTM
- Jerry Chen
On April 5, 2016, 6:28 a.m., Colin Ma wrote
ntry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
(line 460)
<https://reviews.apache.org/r/45732/#comment190193>
We may want to reconsider what character to use for the new concept name.
- Jerry Chen
On April 5, 2016, 5:49 a.
ry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java
(line 96)
<https://reviews.apache.org/r/45730/#comment190189>
Use null to indicate NO value for the parameter. The same to other instance
below.
apache.org/r/45727/#comment190174>
sentryRolePrivilegesMap: local variable, doen'st have to be so long. It can
be as simple as rolePrivilegesMap.
It also nice to put the code from 2138 to 2153 into method so simplify the
code logic and also reduce levels of code blocks.
- Jer
/sentry_policy_service.thrift
(line 244)
<https://reviews.apache.org/r/45728/#comment190170>
My oppinion authObjectFilter is not quite a good name here. Naming
suggestions:
objectName
objectPath
- Jerry Chen
On April 5, 2016, 3:53 a.m., Colin Ma
ling white space here.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java
(line 139)
<https://reviews.apache.org/r/34087/#comment189575>
Let's remove the trail
556>
The same as to the name "ug".
- Jerry Chen
On May 12, 2015, 7:11 a.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mail. To reply,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34085/#review126527
---
LGTM
- Jerry Chen
On May 12, 2015, 7:08 a.m., Colin Ma wrote
/hdfs/SentryPlugin.java
(line 161)
<https://reviews.apache.org/r/34084/#comment189541>
Complete the todo.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
(line 167)
<https://reviews.apache.org/r/34084/#comment189543>
Complete the tod
et instance for other utility.
- Jerry Chen
On May 12, 2015, 8:19 a.m., Colin Ma wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https:/
licy/sqoop/SimpleSqoopPolicyEngine.java
(line 72)
<https://reviews.apache.org/r/34082/#comment189534>
The same question above as to user support.
- Jerry Chen
On May 12, 2015, 7:01 a.m., Colin Ma wrote:
>
> ---
> This is an automat
/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
(line 237)
<https://reviews.apache.org/r/34081/#comment189522>
Suggest to use "name" instead of desc.getName() since name variable is
there and used by other clauses.
- Jerry Chen
On May 12, 2015, 6:59 a.m.
in/resources/sentry_policy_service.thrift
(line 296)
<https://reviews.apache.org/r/34078/#comment189520>
The same suggestion on using "ug".
Since the names are already long, I may don't have to use shorten here.
- Jerry Chen
On May 12
224)
<https://reviews.apache.org/r/34080/#comment189516>
Can this code block put after the group related code block?
It seems that we follow a pattern to add the code for user after the group.
But this time is reverse?
- Jerry Chen
On May 12, 2015, 6:57 a.
ava
(line 1398)
<https://reviews.apache.org/r/34079/#comment189237>
Check: Do the two calls getRoleNamesForGroups and getRoleNamesForUsers need
to be in the same transaction?
- Jerry Chen
On May 12, 2015, 6:54 a.m., Colin Ma wrote:
>
> ---
40 matches
Mail list logo
