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




intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java (line 72)
<https://reviews.apache.org/r/56184/#comment237719>

    I remember reading recommendations to use Long.valueOf("0"), instead "of 
new Long(0)". Not sure where "0L" stands though..



repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
 (line 255)
<https://reviews.apache.org/r/56184/#comment237720>

    Is this assignment needed?



repository/src/test/java/org/apache/atlas/services/MetricsServiceTest.java 
(line 119)
<https://reviews.apache.org/r/56184/#comment237721>

    Only whitespace changes in this file? please revert.



webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 (line 274)
<https://reviews.apache.org/r/56184/#comment237725>

    Instead of calling atlasEntityStore.updateByUniqueAttributes() with a 
single entity, it will be safer to call 
atlasEntityStore.createOrUpdate(entities , true) - this will handle the cases 
where message.getEntity() has attributes that contain other entities (like 
hive_table.columns)



webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 (line 314)
<https://reviews.apache.org/r/56184/#comment237726>

    the message doesn't look right. Please review.



webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 (line 334)
<https://reviews.apache.org/r/56184/#comment237727>

    the message doesn't look right. Please review.



webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 (line 380)
<https://reviews.apache.org/r/56184/#comment237728>

    Consider moving toAtlasEntityWithExtInfoMap() and fromV1toV2Entity() 
outside of notification processor. Perhaps to AtlasFormatConverter or some util 
class.



webapp/src/main/java/org/apache/atlas/web/adapters/AtlasObjectIdConverter.java 
(line 59)
<https://reviews.apache.org/r/56184/#comment237729>

    Consider having lines #59-64 inside the following block:
    
    if (!converterContext.contains(ret.getGuid()) {
      // ...
    }



webapp/src/main/java/org/apache/atlas/web/adapters/AtlasStructFormatConverter.java
 (line 142)
<https://reviews.apache.org/r/56184/#comment237732>

    this if/else may not be needed. It should be replaced with:
    
      AtlasFormatConverter attrConverter = 
converterRegistry.getConverter(attrType.getTypeCategory());
    
      v1Value = attrConverter.fromV2ToV1(v2Value, attrType, context);
      
    Please ensure that AtlasObjectIdConverter is registered in 
converterRegistry.


- Madhan Neethiraj


On Feb. 16, 2017, 8:41 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56184/
> -----------------------------------------------------------
> 
> (Updated Feb. 16, 2017, 8:41 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj, Sarath Subramanian, Suma 
> Shivaprasad, and Vimal Sharma.
> 
> 
> Bugs: ATLAS-1499
>     https://issues.apache.org/jira/browse/ATLAS-1499
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-1499: Notification processing using V2 Store
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ae6be843 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 
> edaede0c 
>   
> intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java
>  5e8ce351 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
>  1ef803c0 
>   repository/src/test/java/org/apache/atlas/services/MetricsServiceTest.java 
> cf85b2fc 
>   server-api/src/main/java/org/apache/atlas/RequestContextV1.java bf731746 
>   
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
>  f2416810 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasAbstractFormatConverter.java
>  f1f3d18a 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasArrayFormatConverter.java
>  aa14aff4 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasClassificationFormatConverter.java
>  dc740f55 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasEntityFormatConverter.java
>  cb1390d1 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasEnumFormatConverter.java
>  6d8e3aee 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasFormatConverter.java 
> a7157a36 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasFormatConverters.java 
> 968d74fe 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasInstanceRestAdapters.java
>  b1dae56d 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasMapFormatConverter.java
>  6967c4f5 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasObjectIdConverter.java
>  11a020d8 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasPrimitiveFormatConverter.java
>  2b70c5e1 
>   
> webapp/src/main/java/org/apache/atlas/web/adapters/AtlasStructFormatConverter.java
>  920b48b0 
>   webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java 
> 2c2c16d7 
>   webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java 9518f540 
>   webapp/src/test/java/org/apache/atlas/LocalAtlasClientTest.java c5616dfe 
>   
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerKafkaTest.java
>  873e5625 
>   
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java
>  f06f7912 
> 
> Diff: https://reviews.apache.org/r/56184/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipITs executed successfully.
> 
> UI/Manual testing details
> 
> **Changes seen on UI **
> 1. Manually created table in hive, showed up in graph and as an entity from 
> the search UI
> 2. Created a view from the above table and the lineage showed up in atlas 
> dashboard
> 3. added a column to the above table
> 4. altered table columns
> 
> 
> Will be running HiveHookIT to verify more cases.
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Reply via email to