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




catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java 
(line 48)
<https://reviews.apache.org/r/48820/#comment203411>

    This does not need to be synchronized, right? Can we just protect the 
creation of default taxonomy alone? That way, we would maximize on parallelism. 
Please consider this suggestion for all methods that have the 
createDefaultTaxonomyIfNeeded call.



catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java 
(line 81)
<https://reviews.apache.org/r/48820/#comment203414>

    If the intention of checking for a default taxonomy is to ensure that it is 
in place after this method, it would be confusing if a DELETE is called with 
the default taxonomy name as the first call (which is very hypothetical and 
unlikely), but still it contradicts the intent of the check. Can we do the 
default taxonomy creation only in the GET / PUT calls as covered by other 
methods?



catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java 
(line 161)
<https://reviews.apache.org/r/48820/#comment203415>

    If we need to create a specific taxonomy (default) which we are taking from 
the configuration, can't we just lookup for that specifically?



catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java 
(line 165)
<https://reviews.apache.org/r/48820/#comment203416>

    Could you consider using this:
    
    ```
    defaultTaxonomyName = "default";
    try {
      ApplicationProperties.get().getString("...", "default");
    } catch (Exception e) {
      LOG...
    }
    ```
    
    Basically, the check for taxonomy name null or empty can be avoided using 
the config.get call with a default value.


- Hemanth Yamijala


On June 16, 2016, 10:22 p.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48820/
> -----------------------------------------------------------
> 
> (Updated June 16, 2016, 10:22 p.m.)
> 
> 
> Review request for atlas and Hemanth Yamijala.
> 
> 
> Bugs: ATLAS-806
>     https://issues.apache.org/jira/browse/ATLAS-806
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Provide a default taxonomy unless a user created taxonomy already exists.
> 
> 
> Diffs
> -----
> 
>   
> catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java 
> 3a5d9be 
>   
> catalog/src/test/java/org/apache/atlas/catalog/TaxonomyResourceProviderTest.java
>  b833c6e 
>   distro/src/conf/atlas-application.properties cc0e4c1 
> 
> Diff: https://reviews.apache.org/r/48820/diff/
> 
> 
> Testing
> -------
> 
> - Added new unit tests
> - All unit tests pass
> - Functional tests pass
> 
> 
> Thanks,
> 
> John Speidel
> 
>

Reply via email to