-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/#review217310
-----------------------------------------------------------




hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 157 (patched)
<https://reviews.apache.org/r/71322/#comment304601>

    Please consider simplifying this by directly providing the database name as 
an argument to createPoliciesForHiveDB() function (which is private and hence 
visible only in this class).



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 180 (patched)
<https://reviews.apache.org/r/71322/#comment304602>

    Please consider renaming this function as RangerPolicy 
createDatabaseDefaultPolicies(String databaseName);
    
    with return value as newly created policy. THe caller can accumulate 
returned policy into a list.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 183 (patched)
<https://reviews.apache.org/r/71322/#comment304603>

    Please review text of the function entry message.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 188 (patched)
<https://reviews.apache.org/r/71322/#comment304604>

    I assume that Integer.valueOf(0) refers to RangerPolicy.POLICY_TYPE_ACCESS. 
If so, please use the it instead of hard-coding value 0.
    
    Also consider the type of PROP_POLICY_NAME_SUFFIX to be Iterator of correct 
type, such as Iterator<List<RangerServiceDef.RangerResourceDef>>. 
    Then the type casting on the next line will not be required.
    
    Please consider renaming PROP_POLICY_NAME_PREFIX and 
PROP_POLICY_NAME_SUFFIX to more descriptive names.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 191 (patched)
<https://reviews.apache.org/r/71322/#comment304605>

    policyIndexes->resourceDefList



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 201 (patched)
<https://reviews.apache.org/r/71322/#comment304606>

    Please review error message text. Database name is hardcoded there as 
DEFAULT.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 212 (patched)
<https://reviews.apache.org/r/71322/#comment304607>

    Please consider skipping hierarchies that do not contain ResourceDef with 
name "database". In future, if more top-level resources get added to hive 
service-def, such code will not require any change.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 223 (patched)
<https://reviews.apache.org/r/71322/#comment304608>

    Pleare review name of the function.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 225 (patched)
<https://reviews.apache.org/r/71322/#comment304609>

    Please review text of log message to have correct method name.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 288 (patched)
<https://reviews.apache.org/r/71322/#comment304610>

    If the database name is passed to this function, then this code block will 
not be required. Please review.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 319 (patched)
<https://reviews.apache.org/r/71322/#comment304611>

    Why is this code block needed? hiveDatabaseName is not used in the body of 
switch statement anywhere.



hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 340 (patched)
<https://reviews.apache.org/r/71322/#comment304612>

    This may not be needed if the database name passed in to private functions 
in this class.


- Abhay Kulkarni


On Aug. 20, 2019, 12:28 a.m., Ramesh Mani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71322/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2019, 12:28 a.m.)
> 
> 
> Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, 
> Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, 
> and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2539
>     https://issues.apache.org/jira/browse/RANGER-2539
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> RANGER-2539:Create Default Policies for Hive Databases -default, 
> Information_schema
> 
> 
> Diffs
> -----
> 
>   
> hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
>  89b8100 
> 
> 
> Diff: https://reviews.apache.org/r/71322/diff/1/
> 
> 
> Testing
> -------
> 
> Testing in local vm by create a hive service.
> 
> 
> Thanks,
> 
> Ramesh Mani
> 
>

Reply via email to