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
>
>