Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-20 Thread Madhan Neethiraj 

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


Ship it!




Ship It!

- Madhan Neethiraj


On Feb. 20, 2018, 7 a.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65700/
> ---
> 
> (Updated Feb. 20, 2018, 7 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2456
> https://issues.apache.org/jira/browse/ATLAS-2456
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Scalable way to quickly and efficiently propagate tags for efficient searches 
> and tag based security. Likewise tags for derivative dataset should be 
> inherited from the parent. For example, if an entity is tagged "PII" then 
> resulting entity created from a CTAS operation should also be tagged "secret" 
> to maintain the classification of the parent. In the case where 2 or more 
> datasets are aggregated the derivative dataset should be a union of all 
> parent tags.
> 
> This changeset includes introduction of v2 data structures for notifications 
> and audit events.
> 
> 
> Diffs
> -
> 
>   addons/models/-Area0/0010-base_model.json 0296e8f9 
>   addons/models/1000-Hadoop/1030-hive_model.json 32d91793 
>   addons/models/1000-Hadoop/1060-hbase_model.json 9280f599 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 265be788 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
>  42837f49 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertex.java
>  a68d8ebe 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertex.java
>  aef20f03 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Vertex.java
>  ca48e3d9 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ff09e6c9 
>   intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> f594a814 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 
> 08d1ce11 
>   
> intg/src/main/java/org/apache/atlas/model/notification/EntityNotification.java
>  b272b733 
>   
> intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
>  74d3b913 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditRepository.java
>  9dc78350 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
>  774934c7 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java
>  22d2a810 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/NoopEntityAuditRepository.java
>  c3826019 
>   
> repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java
>  2884f8f0 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  31620b1e 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d61bff24 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java
>  2b6beade 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
>  ca0eeeb6 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
>  7389f49b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
>  779bc387 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
>  b05a9a32 
>   
> repository/src/main/java/org/apache/atlas/util/AtlasGremlin3QueryProvider.java
>  1fda2415 
>   
> repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 
> a79abaaf 
>   
> repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java
>  0e1e5b6c 
>   repository/src/test/java/org/apache/atlas/TestModules.java 13bdcb08 
>   
> repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryTestBase.java
>  47a61ee2 
>   
> webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java
>  PRE-CREATION 
>   
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java
>  396a2920 
>

Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-20 Thread Sarath Subramanian 


> On Feb. 19, 2018, 12:03 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
> > Lines 574 (patched)
> > 
> >
> > In edges that connect an entity vertex with a classification vertex, 
> > would adding a property help faster retrieval of classifications of an 
> > entity?
> > classificationKind: values= { ASSOCIATED, PROPAGATED }
> > 
> > 
> > To retrieve:
> >  - all classifications, follow edges that have the property 
> > "classificationKind"
> >  - propagated classifications, follow edges that have property value 
> > "classificationKind=PROPAGATED"
> >  - directly associated classifications, follow edges that have property 
> > value "classificationKind=ASSOCIATED"

Good point, adding this information in edge will help fast retrieval. I will 
add this optimization in a follow-up patch.


- Sarath


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


On Feb. 19, 2018, 11 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65700/
> ---
> 
> (Updated Feb. 19, 2018, 11 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2456
> https://issues.apache.org/jira/browse/ATLAS-2456
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Scalable way to quickly and efficiently propagate tags for efficient searches 
> and tag based security. Likewise tags for derivative dataset should be 
> inherited from the parent. For example, if an entity is tagged "PII" then 
> resulting entity created from a CTAS operation should also be tagged "secret" 
> to maintain the classification of the parent. In the case where 2 or more 
> datasets are aggregated the derivative dataset should be a union of all 
> parent tags.
> 
> This changeset includes introduction of v2 data structures for notifications 
> and audit events.
> 
> 
> Diffs
> -
> 
>   addons/models/-Area0/0010-base_model.json 0296e8f9 
>   addons/models/1000-Hadoop/1030-hive_model.json 32d91793 
>   addons/models/1000-Hadoop/1060-hbase_model.json 9280f599 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 265be788 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
>  42837f49 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertex.java
>  a68d8ebe 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertex.java
>  aef20f03 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Vertex.java
>  ca48e3d9 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ff09e6c9 
>   intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> f594a814 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 
> 08d1ce11 
>   
> intg/src/main/java/org/apache/atlas/model/notification/EntityNotification.java
>  b272b733 
>   
> intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
>  74d3b913 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditRepository.java
>  9dc78350 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
>  774934c7 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java
>  22d2a810 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/NoopEntityAuditRepository.java
>  c3826019 
>   
> repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java
>  2884f8f0 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  31620b1e 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d61bff24 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java
>  2b6beade 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
>  ca0eeeb6 
>   
>

Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-19 Thread Madhan Neethiraj 

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




intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
Lines 140 (patched)


"00G" ==> "010"



intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
Lines 161 (patched)


"ATLAS-500-00-00B" is already used in line #163 below. Please review and 
update.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1532 (patched)


This 'if' block would result in notification not to be sent for propagated 
entities. An entity notificaiton should sent for any change in classifications 
& propagated-classification.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1546 (patched)


Verify that the classification is applicable for the entity-type.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 574 (patched)


In edges that connect an entity vertex with a classification vertex, would 
adding a property help faster retrieval of classifications of an entity?
classificationKind: values= { ASSOCIATED, PROPAGATED }

To retrieve:
 - all classifications, follow edges that have the property 
"classificationKind"
 - propagated classifications, follow edges that have property value 
"classificationKind=PROPAGATED"
 - directly associated classifications, follow edges that have property 
value "classificationKind=ASSOCIATED"


- Madhan Neethiraj


On Feb. 18, 2018, 6:10 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65700/
> ---
> 
> (Updated Feb. 18, 2018, 6:10 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2456
> https://issues.apache.org/jira/browse/ATLAS-2456
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Scalable way to quickly and efficiently propagate tags for efficient searches 
> and tag based security. Likewise tags for derivative dataset should be 
> inherited from the parent. For example, if an entity is tagged "PII" then 
> resulting entity created from a CTAS operation should also be tagged "secret" 
> to maintain the classification of the parent. In the case where 2 or more 
> datasets are aggregated the derivative dataset should be a union of all 
> parent tags.
> 
> This changeset includes introduction of v2 data structures for notifications 
> and audit events.
> 
> 
> Diffs
> -
> 
>   addons/models/-Area0/0010-base_model.json 0296e8f9 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 265be788 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
>  42837f49 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertex.java
>  a68d8ebe 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertex.java
>  aef20f03 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Vertex.java
>  ca48e3d9 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ff09e6c9 
>   intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> f594a814 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 
> 08d1ce11 
>   
> intg/src/main/java/org/apache/atlas/model/notification/EntityNotification.java
>  b272b733 
>   
> intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
>  74d3b913 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditRepository.java
>  9dc78350 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
>  774934c7 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java
>  22d2a810 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/NoopEntityAuditRepository.java
>  c3826019 
>   
>

Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-18 Thread Sarath Subramanian 

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

(Updated Feb. 18, 2018, 10:10 a.m.)


Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.


Changes
---

addressed review comments.


Bugs: ATLAS-2456
https://issues.apache.org/jira/browse/ATLAS-2456


Repository: atlas


Description
---

Scalable way to quickly and efficiently propagate tags for efficient searches 
and tag based security. Likewise tags for derivative dataset should be 
inherited from the parent. For example, if an entity is tagged "PII" then 
resulting entity created from a CTAS operation should also be tagged "secret" 
to maintain the classification of the parent. In the case where 2 or more 
datasets are aggregated the derivative dataset should be a union of all parent 
tags.

This changeset includes introduction of v2 data structures for notifications 
and audit events.


Diffs (updated)
-

  addons/models/-Area0/0010-base_model.json 0296e8f9 
  common/src/main/java/org/apache/atlas/repository/Constants.java 265be788 
  dashboardv2/public/js/utils/Enums.js a7b9a8b6 
  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java 
42837f49 
  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertex.java 
a68d8ebe 
  
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertex.java
 aef20f03 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Vertex.java
 ca48e3d9 
  intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ff09e6c9 
  intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
PRE-CREATION 
  intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
PRE-CREATION 
  intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
f594a814 
  intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 08d1ce11 
  
intg/src/main/java/org/apache/atlas/model/notification/EntityNotification.java 
b272b733 
  
intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
 PRE-CREATION 
  
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
 74d3b913 
  
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
 PRE-CREATION 
  
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditRepository.java
 9dc78350 
  
repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
 774934c7 
  
repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java
 22d2a810 
  
repository/src/main/java/org/apache/atlas/repository/audit/NoopEntityAuditRepository.java
 c3826019 
  
repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java
 2884f8f0 
  
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
 31620b1e 
  repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
d61bff24 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
 79e8e3e8 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java
 2b6beade 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
 ca0eeeb6 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
 7389f49b 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 779bc387 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
 b05a9a32 
  
repository/src/main/java/org/apache/atlas/util/AtlasGremlin3QueryProvider.java 
1fda2415 
  repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 
a79abaaf 
  
repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java
 0e1e5b6c 
  repository/src/test/java/org/apache/atlas/TestModules.java 13bdcb08 
  
repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryTestBase.java
 47a61ee2 
  
webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java
 PRE-CREATION 
  
webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java
 396a2920 
  webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
715a54db 
  webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java 21402e19 
  webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java 
ea6fe313 
  webapp/src/test/resources/atlas-application.properties d9009166 


Diff: https://reviews.apache.org/r/65700/diff/3/

Changes: https://reviews.apache.org/r/65700/diff/2-3/


Testing
---

mvn clean install -> succeeded in 17:11 min

UTs/ITs for tag

Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-18 Thread David Radley 

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




addons/models/-Area0/0010-base_model.json
Line 216 (original), 216 (patched)


you are adding tag propagation from a DataSet to a Process and from a 
Process to a dataset. I realise these are aggregation relationships, I wonder 
what prevents circularities.



common/src/main/java/org/apache/atlas/repository/Constants.java
Lines 72 (patched)


Can we use classification instead of traits or tags?



dashboardv2/public/js/utils/Enums.js
Lines 31 (patched)


agree with the previous reviewer that these tags seem the same as the TAG_ 
enums



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1141 (patched)


I wonder if you could explain what ths creation of a propagation edge 
means. This implies that the propagated classifications are stored in the 
edges. I am wondering how this fits with associating the propagated 
classifications with the downstream entities, which I assume will be stored as 
propagation classifications in the downstream entities - have /i understood 
this correctly. A comment to explain how this works would be helpful.


- David Radley


On Feb. 18, 2018, 8:27 a.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65700/
> ---
> 
> (Updated Feb. 18, 2018, 8:27 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2456
> https://issues.apache.org/jira/browse/ATLAS-2456
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Scalable way to quickly and efficiently propagate tags for efficient searches 
> and tag based security. Likewise tags for derivative dataset should be 
> inherited from the parent. For example, if an entity is tagged "PII" then 
> resulting entity created from a CTAS operation should also be tagged "secret" 
> to maintain the classification of the parent. In the case where 2 or more 
> datasets are aggregated the derivative dataset should be a union of all 
> parent tags.
> 
> This changeset includes introduction of v2 data structures for notifications 
> and audit events.
> 
> 
> Diffs
> -
> 
>   addons/models/-Area0/0010-base_model.json 0296e8f9 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 265be788 
>   dashboardv2/public/js/utils/Enums.js a7b9a8b6 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasElement.java
>  42837f49 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertex.java
>  a68d8ebe 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertex.java
>  aef20f03 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Vertex.java
>  ca48e3d9 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ff09e6c9 
>   intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> f594a814 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 
> 08d1ce11 
>   
> intg/src/main/java/org/apache/atlas/model/notification/EntityNotification.java
>  b272b733 
>   
> intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
>  74d3b913 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditRepository.java
>  9dc78350 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
>  774934c7 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java
>  22d2a810 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/NoopEntityAuditRepository.java
>  c3826019 
>   
> repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java
>  2884f8f0 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/FullTextMapperV2.java
>  c42aa156 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  31620b1e 
>

Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-18 Thread Sarath Subramanian 


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > dashboardv2/public/js/utils/Enums.js
> > Lines 31 (patched)
> > 
> >
> > How are these different from existing enums 
> > TAG_ADD/TAG_DELETE/TAG_UPDATE?

If v2 structure is used for audit events, audit action is updated to use 
CLASSIFICATION instead of TAG:
TAG_ADD => CLASSIFICATION_ADD ...


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/audit/EntityAuditEventV2.java
> > Lines 44 (patched)
> > 
> >
> > Consider moving this class to "models" package, as this is referenced 
> > in REST APIs.

moved to package org.apache.atlas.model.audit


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
> > Lines 57 (patched)
> > 
> >
> > 'entity' already has fields classifications & 
> > propagatedClassifications. does 'allClassifications' include both these 
> > lists, and also have super-type classification instances?

Yes, allClassifications include both classifications, propagatedClassifications 
and all its supertypes information. Its computed in 
EntityNotificationListenerV2.getAllClassifications() method.


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
> > Lines 439 (patched)
> > 
> >
> > Multiple edges for the given classification name can exists - consider 
> > multiple classifications of the same name propagated from different 
> > entities. To handle such cases, all the edges returned should be iterated.

For a given classification name, only one edge exists from an entity. The same 
classification propagated from different entities will have a different edge 
label with prefix - "propagated:classificationName". e.g: entity can have one 
'PII' with edge label 'PII' but multiple propagated 'PII' with edge label 
'propagated:PII'. Anyway this is called from an unused method, will remove it.


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
> > Lines 1125 (patched)
> > 
> >
> > Is this call to getAllTraitNames() necessary here? Query in this call 
> > is duplocated in the call to getAllClassificationVertices() below - line 
> > #1128.

getAllTraitNames() will read from the vertex ('fromVertex') to check if any 
tags exists, instead of traversing the edges to find for tag vertices. If the 
vertex property doesn't have any values (classifications/propagated 
classifications), we can avoid traversal to edges to find if tag vertices 
exists.


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
> > Lines 1180 (patched)
> > 
> >
> > Is this call to getAllTraitNames() necessary here? Query in this call 
> > is duplocated in the call to getAllClassificationVertices() below - line 
> > #1183.

same as addTagPropagation reason - getAllTraitNames() will read from the vertex 
('fromVertex') to check if any tags exists, instead of traversing the edges to 
find for tag vertices - will help in failing-fast.


> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
> > Lines 1232 (patched)
> > 
> >
> > Multiple classification instances of a given name can be in propagated 
> > to an entity. Please review if removePropagatedTraitNameFromVertex() 
> > handles this case correctly.

yes, this method removes the first occurrence of the classification name from 
the propagatedTraitNames property, if multiple trait names of the same name 
exists.


- Sarath


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


On Feb. 18, 2018, 12:27 a.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65700/
> ---
> 
> (Updated Feb. 18, 2018, 12:27 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan

Re: Review Request 65700: [ATLAS-2456]: Implement tag propagation using relationships

2018-02-17 Thread Madhan Neethiraj 

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




dashboardv2/public/js/utils/Enums.js
Lines 31 (patched)


How are these different from existing enums TAG_ADD/TAG_DELETE/TAG_UPDATE?



graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasVertex.java
Lines 59 (patched)


"If the property is already present, it is added again and no exception is 
thrown." - perhaps it should be:
"if the property is present, the given value will be added to its existing 
value. If property is not present, it will be created with the given value"



intg/src/main/java/org/apache/atlas/audit/EntityAuditEventV2.java
Lines 44 (patched)


Consider moving this class to "models" package, as this is referenced in 
REST APIs.



intg/src/main/java/org/apache/atlas/audit/EntityAuditEventV2.java
Lines 55 (patched)


What is the "details" field used for? If this is not necessary, consider 
removing it.



intg/src/main/java/org/apache/atlas/audit/EntityAuditEventV2.java
Lines 57 (patched)


"entityDefinition" ==> "entity"



intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java
Lines 55 (patched)


Is it necessary to assign an internal-guid for classifications? Consider 
setting this to null by default.



intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
Lines 271 (patched)


Consider inserting the following line after #270 and removing #273:
   this.classificaitons = c;



intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
Lines 282 (patched)


Similar to addClassifications(), consider adding method 
addPropagatedClassifications().



intg/src/main/java/org/apache/atlas/v1/model/notification/EntityNotificationV2.java
Lines 57 (patched)


'entity' already has fields classifications & propagatedClassifications. 
does 'allClassifications' include both these lists, and also have super-type 
classification instances?



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 439 (patched)


Multiple edges for the given classification name can exists - consider 
multiple classifications of the same name propagated from different entities. 
To handle such cases, all the edges returned should be iterated.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 457 (patched)


If vertex doesn't include CLASSIFICATION_PROPAGATE_KEY property, this line 
might hit NPE. Consider assigning the return from 
AtlasGraphUtilsV1.getProperty() to a Boolean value; and treat null as "true".



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1125 (patched)


Is this call to getAllTraitNames() necessary here? Query in this call is 
duplocated in the call to getAllClassificationVertices() below - line #1128.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1135 (patched)


LOG.info ==> LOG.debug, within LOG.isDebugEnabled().

Please update other such occurances as well.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1138 (patched)


For efficiency, consider moving lines #1138-#1140 after the 'if' block at 
#1142 i.e. if the classification is not to be propagated, no need to retrieve 
these details.

Same is application for removeTagPropagation() method below as well.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1147 (patched)


Consider moving this 'if' bock just before this 'for' loop i.e. when 
impactedEntityVertices is empty, there is no need to enter this 'for' loop.

Same is application for removeTagPropagation() method below as well.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1155 (patched)


Given there will be only one edge between a