atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Madhan Neethiraj <mad...@apache.org>
Subject Re: Review Request 56184: ATLAS-1499: Notification processing using V2 Store
Date Thu, 16 Feb 2017 22:48:18 GMT

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


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