You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by John Speidel <js...@hortonworks.com> on 2016/06/16 22:22:28 UTC

Review Request 48820: Create default taxonomy if required

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

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


Re: Review Request 48820: Create default taxonomy if required

Posted by Hemanth Yamijala <yh...@gmail.com>.
-----------------------------------------------------------
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
> 
>