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 2018/09/12 00:11:31 UTC
Review Request 68692: Improvement to AddClassification
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/
-----------------------------------------------------------
Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Bugs: ATLAS-2870
https://issues.apache.org/jira/browse/ATLAS-2870
Repository: atlas
Description
-------
**Approach**
- Additional parameters to _addClassification_ transform.
- Updated _ImportTransformShaper_ to process _ExportRequest_.
**CURL**
curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
_import-options.json_ with new _addClassification_
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
"replicateFrom": "Cl1"
}
}
```
_import-options.json_ without the new option.
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
"replicateFrom": "Cl1"
}
}
```
Diffs
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
Diff: https://reviews.apache.org/r/68692/diff/1/
Testing
-------
**Unit test**
New test added.
**Functional tests**
Verification via UI.
Thanks,
Ashutosh Mestry
Re: Review Request 68692: Improvement to AddClassification
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/#review208539
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java
Line 74 (original), 74 (patched)
<https://reviews.apache.org/r/68692/#comment292566>
per line #73 above, params could be null. That will result in NPE here in line #74. Please review and update.
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java
Lines 74 (patched)
<https://reviews.apache.org/r/68692/#comment292567>
It might be simpler to add request.getItemsToExport() ('AtlasObjectId' objects) in AddClassification filter. Later have AddClassification look for incoming AtlasEntity to match with any of these AtlasObjectIds. And avoid AddClassification.Tuple3.
boolean skipAddClassification(AtlasEntity entity) {
boolean ret = true;
if (scope.equals(FILTER_SCOPE_TOP_LEVEL) {
for (AtlasObjectId objId : topLevelObjects) {
if (isMatch(entity, objId)) {
ret = false; // its a top-level entity; don't skip
break;
}
}
} else {
ret = false; // add to all entities; don't skip
}
return ret;
}
- Madhan Neethiraj
On Sept. 12, 2018, 12:11 a.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68692/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2018, 12:11 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2870
> https://issues.apache.org/jira/browse/ATLAS-2870
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> - Additional parameters to _addClassification_ transform.
> - Updated _ImportTransformShaper_ to process _ExportRequest_.
>
> **CURL**
>
> curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
>
> _import-options.json_ with new _addClassification_
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
> _import-options.json_ without the new option.
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
>
>
> Diff: https://reviews.apache.org/r/68692/diff/1/
>
>
> Testing
> -------
>
> **Unit test**
> New test added.
>
> **Functional tests**
> Verification via UI.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 68692: Improvement to AddClassification
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/#review208544
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java
Lines 216 (patched)
<https://reviews.apache.org/r/68692/#comment292568>
- shouldn't typeName be checked as well for match?
- to handle null values, replace "entity.getAttribute(entry.getKey()).equals(entry.getValue())" with:
Objects.equals(entity.getAttribute(entry.getKey()), entry.getValue())
- Madhan Neethiraj
On Sept. 12, 2018, 4:26 a.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68692/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2018, 4:26 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2870
> https://issues.apache.org/jira/browse/ATLAS-2870
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> - Additional parameters to _addClassification_ transform.
> - Updated _ImportTransformShaper_ to process _ExportRequest_.
>
> **CURL**
>
> curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
>
> _import-options.json_ with new _addClassification_
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
> _import-options.json_ without the new option.
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
>
>
> Diff: https://reviews.apache.org/r/68692/diff/2/
>
>
> Testing
> -------
>
> **Unit test**
> New test added.
>
> **Functional tests**
> Verification via UI.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 68692: Improvement to AddClassification
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/#review208545
-----------------------------------------------------------
Fix it, then Ship it!
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java
Lines 217 (patched)
<https://reviews.apache.org/r/68692/#comment292569>
ret = Objects.equals(objectId.getTypeName(), entity.getTypeName());
if (ret) {
for (Map.Entry<String, Object> entry : objectId.getUniqueAttributes().entrySet()) {
ret = Objects.equals(entity.getAttribute(entry.getKey()), entry.getValue());
if (!ret) {
break;
}
}
}
- Madhan Neethiraj
On Sept. 12, 2018, 4:41 a.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68692/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2018, 4:41 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2870
> https://issues.apache.org/jira/browse/ATLAS-2870
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> - Additional parameters to _addClassification_ transform.
> - Updated _ImportTransformShaper_ to process _ExportRequest_.
>
> **CURL**
>
> curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
>
> _import-options.json_ with new _addClassification_
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
> _import-options.json_ without the new option.
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
>
>
> Diff: https://reviews.apache.org/r/68692/diff/3/
>
>
> Testing
> -------
>
> **Unit test**
> New test added.
>
> **Functional tests**
> Verification via UI.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 68692: Improvement to AddClassification
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/#review208547
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On Sept. 12, 2018, 5:02 a.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68692/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2018, 5:02 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2870
> https://issues.apache.org/jira/browse/ATLAS-2870
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> - Additional parameters to _addClassification_ transform.
> - Updated _ImportTransformShaper_ to process _ExportRequest_.
>
> **CURL**
>
> curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
>
> _import-options.json_ with new _addClassification_
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
> _import-options.json_ without the new option.
> ```
> {
> "options": {
> "transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
> "replicateFrom": "Cl1"
> }
> }
>
> ```
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
> repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
>
>
> Diff: https://reviews.apache.org/r/68692/diff/4/
>
>
> Testing
> -------
>
> **Unit test**
> New test added.
>
> **Functional tests**
> Verification via UI.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 68692: Improvement to AddClassification
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/
-----------------------------------------------------------
(Updated Sept. 12, 2018, 5:02 a.m.)
Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Updates include: Addressed review comments.
Bugs: ATLAS-2870
https://issues.apache.org/jira/browse/ATLAS-2870
Repository: atlas
Description
-------
**Approach**
- Additional parameters to _addClassification_ transform.
- Updated _ImportTransformShaper_ to process _ExportRequest_.
**CURL**
curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
_import-options.json_ with new _addClassification_
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
"replicateFrom": "Cl1"
}
}
```
_import-options.json_ without the new option.
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
"replicateFrom": "Cl1"
}
}
```
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
Diff: https://reviews.apache.org/r/68692/diff/4/
Changes: https://reviews.apache.org/r/68692/diff/3-4/
Testing
-------
**Unit test**
New test added.
**Functional tests**
Verification via UI.
Thanks,
Ashutosh Mestry
Re: Review Request 68692: Improvement to AddClassification
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/
-----------------------------------------------------------
(Updated Sept. 12, 2018, 4:41 a.m.)
Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Updates include: Addressed review comments.
Bugs: ATLAS-2870
https://issues.apache.org/jira/browse/ATLAS-2870
Repository: atlas
Description
-------
**Approach**
- Additional parameters to _addClassification_ transform.
- Updated _ImportTransformShaper_ to process _ExportRequest_.
**CURL**
curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
_import-options.json_ with new _addClassification_
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
"replicateFrom": "Cl1"
}
}
```
_import-options.json_ without the new option.
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
"replicateFrom": "Cl1"
}
}
```
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
Diff: https://reviews.apache.org/r/68692/diff/3/
Changes: https://reviews.apache.org/r/68692/diff/2-3/
Testing
-------
**Unit test**
New test added.
**Functional tests**
Verification via UI.
Thanks,
Ashutosh Mestry
Re: Review Request 68692: Improvement to AddClassification
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68692/
-----------------------------------------------------------
(Updated Sept. 12, 2018, 4:26 a.m.)
Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Updates include: Addressed review comments.
Bugs: ATLAS-2870
https://issues.apache.org/jira/browse/ATLAS-2870
Repository: atlas
Description
-------
**Approach**
- Additional parameters to _addClassification_ transform.
- Updated _ImportTransformShaper_ to process _ExportRequest_.
**CURL**
curl -g -X POST -u admin:admin -H "Content-Type: multipart/form-data" -H "Cache-Control: no-cache" -F request=@../docs/import-options.json -F data=@../docs/stocks-2.zip "http://localhost:21000/api/atlas/admin/import"
_import-options.json_ with new _addClassification_
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2:topLevel\" ] } }",
"replicateFrom": "Cl1"
}
}
```
_import-options.json_ without the new option.
```
{
"options": {
"transforms": "{ \"Asset\": { \"*\":[ \"addClassification:REPLICATED_2\" ] } }",
"replicateFrom": "Cl1"
}
}
```
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java 095f60f5c
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformer.java 213539d59
repository/src/main/java/org/apache/atlas/repository/impexp/ImportTransformsShaper.java 62eba455f
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsTest.java cd623d08a
Diff: https://reviews.apache.org/r/68692/diff/2/
Changes: https://reviews.apache.org/r/68692/diff/1-2/
Testing
-------
**Unit test**
New test added.
**Functional tests**
Verification via UI.
Thanks,
Ashutosh Mestry