> On Feb. 17, 2018, 9:10 p.m., Madhan Neethiraj wrote:
> > dashboardv2/public/js/utils/Enums.js
> > Lines 31 (patched)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962305#file1962305line31>
> >
> >     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)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962311#file1962311line44>
> >
> >     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)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962316#file1962316line57>
> >
> >     '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)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962332#file1962332line439>
> >
> >     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)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962332#file1962332line1132>
> >
> >     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)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962332#file1962332line1187>
> >
> >     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)
> > <https://reviews.apache.org/r/65700/diff/1/?file=1962332#file1962332line1239>
> >
> >     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 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/0000-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 
>   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/2/
> 
> 
> Testing
> -------
> 
> mvn clean install -> succeeded in 17:11 min
> 
> UTs/ITs for tag propagation work in progress
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>

Reply via email to