atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hemanth Yamijala <yhema...@gmail.com>
Subject Re: Review Request 48533: Business Catalog Update
Date Fri, 10 Jun 2016 05:42:32 GMT

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




catalog/src/main/java/org/apache/atlas/catalog/BaseRequest.java (line 41)
<https://reviews.apache.org/r/48533/#comment202080>

    Minor nit: the double braces seems like a typo (was there in earlier code as well).



catalog/src/main/java/org/apache/atlas/catalog/BaseRequest.java (line 85)
<https://reviews.apache.org/r/48533/#comment202083>

    Should updateProperties also be in the equals / hashCode implementation?



catalog/src/main/java/org/apache/atlas/catalog/BaseResourceProvider.java (line 55)
<https://reviews.apache.org/r/48533/#comment202084>

    This method seems specific to Taxonomy, however BaseResourceProvider handles all types
of entities. So, should this possibly be in a different place. If only few (but more than
one) code paths need it and we want to avoid duplication - can we use some form of composition
to address the issue?



catalog/src/main/java/org/apache/atlas/catalog/TermResourceProvider.java (line 120)
<https://reviews.apache.org/r/48533/#comment202085>

    If this condition does not hold, should we even execute the term query? IOW, can we move
the check upwards before the term query as well.



catalog/src/main/java/org/apache/atlas/catalog/TermResourceProvider.java (line 132)
<https://reviews.apache.org/r/48533/#comment202086>

    If I understand correctly, we wouldn't update anything if it is not description. Should
this then be indicated via a failure / exception etc?


- Hemanth Yamijala


On June 10, 2016, 3:38 a.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48533/
> -----------------------------------------------------------
> 
> (Updated June 10, 2016, 3:38 a.m.)
> 
> 
> Review request for atlas and Hemanth Yamijala.
> 
> 
> Bugs: ATLAS-794
>     https://issues.apache.org/jira/browse/ATLAS-794
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Allows for updating of taxonomy and term property values, excluding the name property.
> 
> 
> Diffs
> -----
> 
>   catalog/src/main/java/org/apache/atlas/catalog/AtlasTypeSystem.java 3a58488 
>   catalog/src/main/java/org/apache/atlas/catalog/BaseRequest.java 9ccb4e3 
>   catalog/src/main/java/org/apache/atlas/catalog/BaseResourceProvider.java ad2f7f4 
>   catalog/src/main/java/org/apache/atlas/catalog/CollectionRequest.java b1be1ae 
>   catalog/src/main/java/org/apache/atlas/catalog/DefaultTypeSystem.java a28a32b 
>   catalog/src/main/java/org/apache/atlas/catalog/EntityResourceProvider.java c8d6f68

>   catalog/src/main/java/org/apache/atlas/catalog/EntityTagResourceProvider.java f73f80b

>   catalog/src/main/java/org/apache/atlas/catalog/InstanceRequest.java 01583c4 
>   catalog/src/main/java/org/apache/atlas/catalog/Request.java 7dc781a 
>   catalog/src/main/java/org/apache/atlas/catalog/ResourceProvider.java 9c809c0 
>   catalog/src/main/java/org/apache/atlas/catalog/TaxonomyResourceProvider.java 0d63336

>   catalog/src/main/java/org/apache/atlas/catalog/TermResourceProvider.java 0c72de6 
>   catalog/src/main/java/org/apache/atlas/catalog/VertexWrapper.java 6e5d28e 
>   catalog/src/main/java/org/apache/atlas/catalog/definition/BaseResourceDefinition.java
32d6f30 
>   catalog/src/main/java/org/apache/atlas/catalog/definition/EntityResourceDefinition.java
cf55f1f 
>   catalog/src/main/java/org/apache/atlas/catalog/definition/ResourceDefinition.java f310c5a

>   catalog/src/main/java/org/apache/atlas/catalog/definition/TaxonomyResourceDefinition.java
a3fbdf1 
>   catalog/src/main/java/org/apache/atlas/catalog/definition/TermResourceDefinition.java
19dd049 
>   catalog/src/main/java/org/apache/atlas/catalog/query/AtlasEntityQuery.java c24b99a

>   catalog/src/main/java/org/apache/atlas/catalog/query/AtlasEntityTagQuery.java df216c0

>   catalog/src/main/java/org/apache/atlas/catalog/query/AtlasQuery.java af14697 
>   catalog/src/main/java/org/apache/atlas/catalog/query/AtlasTermQuery.java b761dcc 
>   catalog/src/main/java/org/apache/atlas/catalog/query/BaseQuery.java ba8e0e7 
>   catalog/src/main/java/org/apache/atlas/catalog/query/QueryFactory.java 39ce11a 
>   catalog/src/test/java/org/apache/atlas/catalog/CollectionRequestTest.java 0a2bace 
>   catalog/src/test/java/org/apache/atlas/catalog/EntityResourceProviderTest.java 2f29103

>   catalog/src/test/java/org/apache/atlas/catalog/EntityTagResourceProviderTest.java 78204a6

>   catalog/src/test/java/org/apache/atlas/catalog/InstanceRequestTest.java 5ccec02 
>   catalog/src/test/java/org/apache/atlas/catalog/TaxonomyResourceProviderTest.java a714a8c

>   catalog/src/test/java/org/apache/atlas/catalog/TermResourceProviderTest.java 235bde4

>   catalog/src/test/java/org/apache/atlas/catalog/definition/EntityResourceDefinitionTest.java
303e2ba 
>   catalog/src/test/java/org/apache/atlas/catalog/definition/EntityTagResourceDefinitionTest.java
954262f 
>   catalog/src/test/java/org/apache/atlas/catalog/definition/TaxonomyResourceDefinitionTest.java
bc6f74c 
>   catalog/src/test/java/org/apache/atlas/catalog/definition/TermResourceDefinitionTest.java
52deadf 
>   catalog/src/test/java/org/apache/atlas/catalog/query/AtlasEntityQueryTest.java 149134c

>   webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java 2a65538 
>   webapp/src/main/java/org/apache/atlas/web/resources/TaxonomyService.java f995198 
>   webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java a967805

> 
> Diff: https://reviews.apache.org/r/48533/diff/
> 
> 
> Testing
> -------
> 
> Added new unit tests.
> All unit tests pass.
> Manual functional tests including create/update/delete/get of taxonomies/terms/tags.
> 
> 
> Thanks,
> 
> John Speidel
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message