You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Ashutosh Mestry <am...@hortonworks.com> on 2017/04/05 21:51:20 UTC
Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/
-----------------------------------------------------------
Review request for atlas and Madhan Neethiraj.
Bugs: ATLAS-1721
https://issues.apache.org/jira/browse/ATLAS-1721
Repository: atlas
Description
-------
**Background**
After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
**Approach**
The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
E.g.
```java
void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
```
Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
**Specifics**
_EntityAuditAction_ is modified to include additional _enum_ values.
Diffs
-----
client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
dashboardv2/public/js/utils/Enums.js c779cd4c
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
Diff: https://reviews.apache.org/r/58222/diff/1/
Testing
-------
Added unit tests for _EntityAuditAction_.
Functional testing:
- Create entity using beeline/hive. Observe the value in _Audit_ tab.
- Perform import. Obeserve value in _Audit_ tab.
Thanks,
Ashutosh Mestry
Re: Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/#review171785
-----------------------------------------------------------
Could you rebase the patch.
- Sarath Subramanian
On April 5, 2017, 2:51 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58222/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 2:51 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
>
>
> Bugs: ATLAS-1721
> https://issues.apache.org/jira/browse/ATLAS-1721
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Background**
> After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
>
> **Approach**
> The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
>
> The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
>
> E.g.
> ```java
> void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
> ```
>
> Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
>
> Modified _Enums.js_ to help UI handle the additional audit values.
>
> **Specifics**
> _EntityAuditAction_ is modified to include additional _enum_ values.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
> client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
> dashboardv2/public/js/utils/Enums.js c779cd4c
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
> repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
>
>
> Diff: https://reviews.apache.org/r/58222/diff/2/
>
>
> Testing
> -------
>
> Added unit tests for _EntityAuditAction_.
>
> Functional testing:
> - Create entity using beeline/hive. Observe the value in _Audit_ tab.
> - Perform import. Obeserve value in _Audit_ tab.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
Posted by Ashutosh Mestry <am...@hortonworks.com>.
> On April 11, 2017, 3:11 a.m., Madhan Neethiraj wrote:
> > webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java
> > Line 80 (original), 80 (patched)
> > <https://reviews.apache.org/r/58222/diff/1/?file=1685551#file1685551line80>
> >
> > Consider using typed & named argument here, like "boolean isImport", instead of variable number of arguments. This will be self-documenting and improve readability.
I had done it this way before, but then its less than ideal for observers who do not find it relevant.
I will implement this.
- Ashutosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/#review171515
-----------------------------------------------------------
On April 5, 2017, 9:51 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58222/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
>
>
> Bugs: ATLAS-1721
> https://issues.apache.org/jira/browse/ATLAS-1721
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Background**
> After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
>
> **Approach**
> The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
>
> The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
>
> E.g.
> ```java
> void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
> ```
>
> Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
>
> Modified _Enums.js_ to help UI handle the additional audit values.
>
> **Specifics**
> _EntityAuditAction_ is modified to include additional _enum_ values.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
> client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
> dashboardv2/public/js/utils/Enums.js c779cd4c
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
> repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
>
>
> Diff: https://reviews.apache.org/r/58222/diff/1/
>
>
> Testing
> -------
>
> Added unit tests for _EntityAuditAction_.
>
> Functional testing:
> - Create entity using beeline/hive. Observe the value in _Audit_ tab.
> - Perform import. Obeserve value in _Audit_ tab.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/#review171515
-----------------------------------------------------------
webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java
Line 80 (original), 80 (patched)
<https://reviews.apache.org/r/58222/#comment244457>
Consider using typed & named argument here, like "boolean isImport", instead of variable number of arguments. This will be self-documenting and improve readability.
- Madhan Neethiraj
On April 5, 2017, 9:51 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58222/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
>
>
> Bugs: ATLAS-1721
> https://issues.apache.org/jira/browse/ATLAS-1721
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Background**
> After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
>
> **Approach**
> The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
>
> The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
>
> E.g.
> ```java
> void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
> ```
>
> Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
>
> Modified _Enums.js_ to help UI handle the additional audit values.
>
> **Specifics**
> _EntityAuditAction_ is modified to include additional _enum_ values.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
> client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
> dashboardv2/public/js/utils/Enums.js c779cd4c
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
> repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
>
>
> Diff: https://reviews.apache.org/r/58222/diff/1/
>
>
> Testing
> -------
>
> Added unit tests for _EntityAuditAction_.
>
> Functional testing:
> - Create entity using beeline/hive. Observe the value in _Audit_ tab.
> - Perform import. Obeserve value in _Audit_ tab.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/#review171769
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On April 5, 2017, 9:51 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58222/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
>
>
> Bugs: ATLAS-1721
> https://issues.apache.org/jira/browse/ATLAS-1721
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Background**
> After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
>
> **Approach**
> The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
>
> The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
>
> E.g.
> ```java
> void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
> ```
>
> Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
>
> Modified _Enums.js_ to help UI handle the additional audit values.
>
> **Specifics**
> _EntityAuditAction_ is modified to include additional _enum_ values.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
> client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
> dashboardv2/public/js/utils/Enums.js c779cd4c
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
> repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
>
>
> Diff: https://reviews.apache.org/r/58222/diff/2/
>
>
> Testing
> -------
>
> Added unit tests for _EntityAuditAction_.
>
> Functional testing:
> - Create entity using beeline/hive. Observe the value in _Audit_ tab.
> - Perform import. Obeserve value in _Audit_ tab.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
Posted by Ashutosh Mestry <am...@hortonworks.com>.
> On April 11, 2017, 5:13 a.m., Ayub Pathan wrote:
> > client/src/test/java/org/apache/atlas/EntityAuditEventTest.java
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/58222/diff/1/?file=1685543#file1685543line38>
> >
> > Since EntityAuditAction is already imported. Consider using enums directly.
> >
> > Example:
> >
> > ```java
> > Assert.assertEquals(EntityAuditEvent.EntityAuditAction.get(TAG_ADD, false), TAG_ADD);
> > ```
This is just to demonstrate in the unit test that adding _TAG_ADD_ and _TAG_DELETE_ via this method does not produce undesirable side effect.
- Ashutosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/#review171519
-----------------------------------------------------------
On April 5, 2017, 9:51 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58222/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
>
>
> Bugs: ATLAS-1721
> https://issues.apache.org/jira/browse/ATLAS-1721
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Background**
> After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
>
> **Approach**
> The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
>
> The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
>
> E.g.
> ```java
> void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
> ```
>
> Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
>
> Modified _Enums.js_ to help UI handle the additional audit values.
>
> **Specifics**
> _EntityAuditAction_ is modified to include additional _enum_ values.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
> client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
> dashboardv2/public/js/utils/Enums.js c779cd4c
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
> repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
>
>
> Diff: https://reviews.apache.org/r/58222/diff/1/
>
>
> Testing
> -------
>
> Added unit tests for _EntityAuditAction_.
>
> Functional testing:
> - Create entity using beeline/hive. Observe the value in _Audit_ tab.
> - Perform import. Obeserve value in _Audit_ tab.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 58222: Import: Added Audit that Clearly Displays
Entities Affected By Import
Posted by Ayub Pathan <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58222/#review171519
-----------------------------------------------------------
client/src/test/java/org/apache/atlas/EntityAuditEventTest.java
Lines 22 (patched)
<https://reviews.apache.org/r/58222/#comment244464>
Please consider importing the exact class members, not *.
Also, consider the same for the other files as well.
client/src/test/java/org/apache/atlas/EntityAuditEventTest.java
Lines 38 (patched)
<https://reviews.apache.org/r/58222/#comment244459>
Since EntityAuditAction is already imported. Consider using enums directly.
Example:
```java
Assert.assertEquals(EntityAuditEvent.EntityAuditAction.get(TAG_ADD, false), TAG_ADD);
```
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
Line 263 (original), 273 (patched)
<https://reviews.apache.org/r/58222/#comment244466>
This will return "Unknown: " for import actions. Please consider adding the new import action details here.
- Ayub Pathan
On April 5, 2017, 9:51 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58222/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
>
>
> Bugs: ATLAS-1721
> https://issues.apache.org/jira/browse/ATLAS-1721
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Background**
> After a successful import operation, the entities imported do not have audits that clearly identifies their mode of creation.
>
> **Approach**
> The _EntityChangeListener_ implements observer pattern. Existing implementation does not have ability to pass parameter information to the concrete implementation.
>
> The approach modifies the implementation such that it is possible to pass parameters to the concrete implementations. This is done using generic parameterized list.
>
> E.g.
> ```java
> void onEntitiesAdded(Collection<ITypedReferenceableInstance> entities, Object... args) throws AtlasException;
> ```
>
> Within the _EntityAuditListener_, the parameter is read and the appropriate choice of the _EntityAuditAction_ is passed.
>
> Modified _Enums.js_ to help UI handle the additional audit values.
>
> **Specifics**
> _EntityAuditAction_ is modified to include additional _enum_ values.
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/atlas/EntityAuditEvent.java 7eaa5563
> client/src/test/java/org/apache/atlas/EntityAuditEventTest.java PRE-CREATION
> dashboardv2/public/js/utils/Enums.js c779cd4c
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java 3f03c50a
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java 0439adab
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java 32b1ea83
> repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java d9ce8c41
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 7e828a18
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 256e839a
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java 978b21d6
>
>
> Diff: https://reviews.apache.org/r/58222/diff/1/
>
>
> Testing
> -------
>
> Added unit tests for _EntityAuditAction_.
>
> Functional testing:
> - Create entity using beeline/hive. Observe the value in _Audit_ tab.
> - Perform import. Obeserve value in _Audit_ tab.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>