You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Madhan Neethiraj <ma...@apache.org> on 2018/09/21 00:33:46 UTC
Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/
-----------------------------------------------------------
Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
Bugs: ATLAS-2882
https://issues.apache.org/jira/browse/ATLAS-2882
Repository: atlas
Description
-------
- refactored import transformer creation to set context in constructor
- fixed incorrect ObjectIdEquals.matches()
Diffs
-----
intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
Diff: https://reviews.apache.org/r/68792/diff/1/
Testing
-------
Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
Thanks,
Madhan Neethiraj
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208873
-----------------------------------------------------------
Ship it!
Ship It!
- Sarath Subramanian
On Sept. 20, 2018, 5:33 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 20, 2018, 5:33 p.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/1/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208916
-----------------------------------------------------------
Ship it!
Ship It!
- Sarath Subramanian
On Sept. 21, 2018, 4:20 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 4:20 p.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/EntityAttribute.java PRE-CREATION
> intg/src/main/java/org/apache/atlas/entitytransform/HdfsPathEntityHandler.java 1a398eaa9
> intg/src/main/java/org/apache/atlas/entitytransform/HiveColumnEntityHandler.java fca94b61d
> intg/src/main/java/org/apache/atlas/entitytransform/HiveDatabaseEntityHandler.java 8a2e81304
> intg/src/main/java/org/apache/atlas/entitytransform/HiveStorageDescriptorEntityHandler.java 6a7b17b52
> intg/src/main/java/org/apache/atlas/entitytransform/HiveTableEntityHandler.java b008e6ceb
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/main/java/org/apache/atlas/entitytransform/TransformerContext.java a7a77b596
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/3/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/
-----------------------------------------------------------
(Updated Sept. 21, 2018, 11:20 p.m.)
Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
introduced EntityAttribute abstraction, to handle typeNames in attribute get/set
Bugs: ATLAS-2882
https://issues.apache.org/jira/browse/ATLAS-2882
Repository: atlas
Description
-------
- refactored import transformer creation to set context in constructor
- fixed incorrect ObjectIdEquals.matches()
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
intg/src/main/java/org/apache/atlas/entitytransform/EntityAttribute.java PRE-CREATION
intg/src/main/java/org/apache/atlas/entitytransform/HdfsPathEntityHandler.java 1a398eaa9
intg/src/main/java/org/apache/atlas/entitytransform/HiveColumnEntityHandler.java fca94b61d
intg/src/main/java/org/apache/atlas/entitytransform/HiveDatabaseEntityHandler.java 8a2e81304
intg/src/main/java/org/apache/atlas/entitytransform/HiveStorageDescriptorEntityHandler.java 6a7b17b52
intg/src/main/java/org/apache/atlas/entitytransform/HiveTableEntityHandler.java b008e6ceb
intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
intg/src/main/java/org/apache/atlas/entitytransform/TransformerContext.java a7a77b596
intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
Diff: https://reviews.apache.org/r/68792/diff/3/
Changes: https://reviews.apache.org/r/68792/diff/2-3/
Testing
-------
Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
Thanks,
Madhan Neethiraj
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/
-----------------------------------------------------------
(Updated Sept. 21, 2018, 7:21 p.m.)
Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
- removed 'context' from Conditions where it is not needed
- updated to handle typeName specified in attribute specified in actions; added unit tests
Bugs: ATLAS-2882
https://issues.apache.org/jira/browse/ATLAS-2882
Repository: atlas
Description
-------
- refactored import transformer creation to set context in constructor
- fixed incorrect ObjectIdEquals.matches()
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
intg/src/main/java/org/apache/atlas/entitytransform/HdfsPathEntityHandler.java 1a398eaa9
intg/src/main/java/org/apache/atlas/entitytransform/HiveColumnEntityHandler.java fca94b61d
intg/src/main/java/org/apache/atlas/entitytransform/HiveDatabaseEntityHandler.java 8a2e81304
intg/src/main/java/org/apache/atlas/entitytransform/HiveStorageDescriptorEntityHandler.java 6a7b17b52
intg/src/main/java/org/apache/atlas/entitytransform/HiveTableEntityHandler.java b008e6ceb
intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
intg/src/main/java/org/apache/atlas/entitytransform/TransformerContext.java a7a77b596
intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
Diff: https://reviews.apache.org/r/68792/diff/2/
Changes: https://reviews.apache.org/r/68792/diff/1-2/
Testing
-------
Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
Thanks,
Madhan Neethiraj
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208835
-----------------------------------------------------------
Ship it!
Ship It!
- Apoorv Naik
On Sept. 21, 2018, 12:33 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 12:33 a.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/1/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Sept. 21, 2018, 5:28 a.m., Ashutosh Mestry wrote:
> > Test will need to be updated once helper method is added.
Which test? Unit tests run successfully with this update.
> On Sept. 21, 2018, 5:28 a.m., Ashutosh Mestry wrote:
> > intg/src/main/java/org/apache/atlas/entitytransform/Action.java
> > Lines 158 (patched)
> > <https://reviews.apache.org/r/68792/diff/1/?file=2090669#file2090669line160>
> >
> > Unnecessary variable. Consider removing.
added the variable for better readability. Also, another goal is to avoid multiple returns.
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208830
-----------------------------------------------------------
On Sept. 21, 2018, 12:33 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 12:33 a.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/1/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Sept. 21, 2018, 5:28 a.m., Ashutosh Mestry wrote:
> > intg/src/main/java/org/apache/atlas/entitytransform/Action.java
> > Line 91 (original), 98 (patched)
> > <https://reviews.apache.org/r/68792/diff/1/?file=2090669#file2090669line98>
> >
> > Remove context here.
context is needed here, to determine if the entity is of the given typeName in action
> On Sept. 21, 2018, 5:28 a.m., Ashutosh Mestry wrote:
> > intg/src/main/java/org/apache/atlas/entitytransform/Action.java
> > Line 95 (original), 102 (patched)
> > <https://reviews.apache.org/r/68792/diff/1/?file=2090669#file2090669line102>
> >
> > Remove context here.
context is needed here, to determine if the entity is of the given typeName in action
> On Sept. 21, 2018, 5:28 a.m., Ashutosh Mestry wrote:
> > intg/src/main/java/org/apache/atlas/entitytransform/Action.java
> > Line 99 (original), 106 (patched)
> > <https://reviews.apache.org/r/68792/diff/1/?file=2090669#file2090669line106>
> >
> > Remove context here.
context is needed here, to determine if the entity is of the given typeName in action
> On Sept. 21, 2018, 5:28 a.m., Ashutosh Mestry wrote:
> > intg/src/main/java/org/apache/atlas/entitytransform/Action.java
> > Line 103 (original), 110 (patched)
> > <https://reviews.apache.org/r/68792/diff/1/?file=2090669#file2090669line110>
> >
> > Remove context here.
context is needed here, to determine if the entity is of the given typeName in action
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208830
-----------------------------------------------------------
On Sept. 21, 2018, 7:21 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 7:21 p.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/HdfsPathEntityHandler.java 1a398eaa9
> intg/src/main/java/org/apache/atlas/entitytransform/HiveColumnEntityHandler.java fca94b61d
> intg/src/main/java/org/apache/atlas/entitytransform/HiveDatabaseEntityHandler.java 8a2e81304
> intg/src/main/java/org/apache/atlas/entitytransform/HiveStorageDescriptorEntityHandler.java 6a7b17b52
> intg/src/main/java/org/apache/atlas/entitytransform/HiveTableEntityHandler.java b008e6ceb
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/main/java/org/apache/atlas/entitytransform/TransformerContext.java a7a77b596
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/2/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208830
-----------------------------------------------------------
Test will need to be updated once helper method is added.
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Line 87 (original), 94 (patched)
<https://reviews.apache.org/r/68792/#comment293041>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Line 91 (original), 98 (patched)
<https://reviews.apache.org/r/68792/#comment293042>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Line 95 (original), 102 (patched)
<https://reviews.apache.org/r/68792/#comment293043>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Line 99 (original), 106 (patched)
<https://reviews.apache.org/r/68792/#comment293044>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Line 103 (original), 110 (patched)
<https://reviews.apache.org/r/68792/#comment293045>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Lines 158 (patched)
<https://reviews.apache.org/r/68792/#comment293046>
Unnecessary variable. Consider removing.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 82 (original), 91 (patched)
<https://reviews.apache.org/r/68792/#comment293047>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 86 (original), 95 (patched)
<https://reviews.apache.org/r/68792/#comment293048>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 90 (original), 99 (patched)
<https://reviews.apache.org/r/68792/#comment293049>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 94 (original), 103 (patched)
<https://reviews.apache.org/r/68792/#comment293050>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 98 (original), 107 (patched)
<https://reviews.apache.org/r/68792/#comment293051>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 102 (original), 111 (patched)
<https://reviews.apache.org/r/68792/#comment293052>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 106 (original), 115 (patched)
<https://reviews.apache.org/r/68792/#comment293053>
Remove context here.
intg/src/main/java/org/apache/atlas/entitytransform/Condition.java
Line 257 (original), 259 (patched)
<https://reviews.apache.org/r/68792/#comment293054>
Remove context here.
intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java
Line 338 (original), 337 (patched)
<https://reviews.apache.org/r/68792/#comment293055>
Collections.singletonMap are repetitive.
Consider adding helper method:
static AttributeTransform create(String conditionLhs, String conditionRhs, String actionLhs, String actionRhs) {
return new AttributeTransform(Collections.singletonMap(conditionLhs, conditionRhs),
Collections.singletonMap(actionLhs, actionRhs));
}
- Ashutosh Mestry
On Sept. 21, 2018, 12:33 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 12:33 a.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/1/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Sept. 21, 2018, 6:08 a.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/entitytransform/Action.java
> > Line 87 (original), 94 (patched)
> > <https://reviews.apache.org/r/68792/diff/1/?file=2090669#file2090669line94>
> >
> > Though the context is not used in many actions and conditions, I think it is better to keep this consistent across all implementations. Can improve readability (since implementations wouldn't different in few common things - like context).
Removed context from bunch of conditions, where it is not used.
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208832
-----------------------------------------------------------
On Sept. 21, 2018, 7:21 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 7:21 p.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/HdfsPathEntityHandler.java 1a398eaa9
> intg/src/main/java/org/apache/atlas/entitytransform/HiveColumnEntityHandler.java fca94b61d
> intg/src/main/java/org/apache/atlas/entitytransform/HiveDatabaseEntityHandler.java 8a2e81304
> intg/src/main/java/org/apache/atlas/entitytransform/HiveStorageDescriptorEntityHandler.java 6a7b17b52
> intg/src/main/java/org/apache/atlas/entitytransform/HiveTableEntityHandler.java b008e6ceb
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/main/java/org/apache/atlas/entitytransform/TransformerContext.java a7a77b596
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/2/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 68792: ATLAS-2882: refactored import transformer
creation to set context in constructor
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68792/#review208832
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/entitytransform/Action.java
Line 87 (original), 94 (patched)
<https://reviews.apache.org/r/68792/#comment293058>
Though the context is not used in many actions and conditions, I think it is better to keep this consistent across all implementations. Can improve readability (since implementations wouldn't different in few common things - like context).
intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java
Line 338 (original), 337 (patched)
<https://reviews.apache.org/r/68792/#comment293057>
Given this is unit test, such refactoring is not critical; and in many tests, it might be useful to explicitly see the values right here, instead of looking else where.
- Madhan Neethiraj
On Sept. 21, 2018, 12:33 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68792/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2018, 12:33 a.m.)
>
>
> Review request for atlas, Abhishek Kadam, Apoorv Naik, Ashutosh Mestry, keval bhatt, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2882
> https://issues.apache.org/jira/browse/ATLAS-2882
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - refactored import transformer creation to set context in constructor
> - fixed incorrect ObjectIdEquals.matches()
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/entitytransform/Action.java fa1855893
> intg/src/main/java/org/apache/atlas/entitytransform/AtlasEntityTransformer.java e9b2afdf7
> intg/src/main/java/org/apache/atlas/entitytransform/BaseEntityHandler.java dd6c66539
> intg/src/main/java/org/apache/atlas/entitytransform/Condition.java 174b9b48b
> intg/src/main/java/org/apache/atlas/entitytransform/NeedsContext.java 5c16bcfbc
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java c76f95916
>
>
> Diff: https://reviews.apache.org/r/68792/diff/1/
>
>
> Testing
> -------
>
> Pre-commit tests run - https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/678/
>
>
> Thanks,
>
> Madhan Neethiraj
>
>