You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Pinal Shah <pi...@freestoneinfotech.com> on 2022/09/12 13:43:20 UTC
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/
-----------------------------------------------------------
(Updated Sept. 12, 2022, 1:43 p.m.)
Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
Bugs: ATLAS-4671
https://issues.apache.org/jira/browse/ATLAS-4671
Repository: atlas
Description
-------
**Issue** : Basic search has AtlasEntityHeader of each entity in the response.
AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph to get all the header attributes.
**Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
Diffs
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
Diff: https://reviews.apache.org/r/74109/diff/2/
Testing (updated)
-------
Unit testcases added
Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
Thanks,
Pinal Shah
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224688
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On Sept. 14, 2022, 12:11 p.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2022, 12:11 p.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
> **Request**: {
> "excludeDeletedEntities": true,
> "includeSubClassifications": true,
> "includeSubTypes": true,
> "includeClassificationAttributes": true,
> "limit": 25,
> "offset": 0,
> "typeName": "hdfs_path",
> "attributes": ["path", "name"],
> "excludeHeaderAttributes": "true"
> }
>
> **Response**: {
> "queryType": "BASIC",
> "searchParameters": {
> "typeName": "hdfs_path",
> "excludeDeletedEntities": true,
> "includeClassificationAttributes": true,
> "includeSubTypes": true,
> "includeSubClassifications": true,
> "limit": 25,
> "offset": 0,
> "attributes": ["path", "name"]
> },
> "attributes": {
> "name": ["path", "name"],
> "values": [
> ["/data/warehouse/customer", "customer"],
> ["/data/warehouse/sales", "sales"]
> ]
> },
> "approximateCount": 2
> }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java a8fe5a762
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/5/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
> Performance Readings:
> Basic Search on 20K entities, limit 1000
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Jayendra Parab <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224706
-----------------------------------------------------------
Ship it!
Ship It!
- Jayendra Parab
On Sept. 21, 2022, 9:48 a.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2022, 9:48 a.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
> **Working** : This improvement can be seen, if user has **entityTypes**, **excludeHeaderAttributes=true** and **valid entity attributes (not relationship) in 'attributes' field** , in the request payload.
>
> **Request**: {
> "excludeDeletedEntities": true,
> "includeSubClassifications": true,
> "includeSubTypes": true,
> "includeClassificationAttributes": true,
> "limit": 25,
> "offset": 0,
> "typeName": "hdfs_path",
> "attributes": ["path", "name"],
> "excludeHeaderAttributes": "true"
> }
>
> **Response**: {
> "queryType": "BASIC",
> "searchParameters": {
> "typeName": "hdfs_path",
> "excludeDeletedEntities": true,
> "includeClassificationAttributes": true,
> "includeSubTypes": true,
> "includeSubClassifications": true,
> "limit": 25,
> "offset": 0,
> "attributes": ["path", "name"]
> },
> "attributes": {
> "name": ["path", "name"],
> "values": [
> ["/data/warehouse/customer", "customer"],
> ["/data/warehouse/sales", "sales"]
> ]
> },
> "approximateCount": 2
> }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/6/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
> Performance Readings:
> Basic Search on 20K entities, limit 1000
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/
-----------------------------------------------------------
(Updated Sept. 21, 2022, 9:48 a.m.)
Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
Changes
-------
addressed review comments
Bugs: ATLAS-4671
https://issues.apache.org/jira/browse/ATLAS-4671
Repository: atlas
Description (updated)
-------
**Issue** : Basic search has AtlasEntityHeader of each entity in the response.
AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
**Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
**Working** : This improvement can be seen, if user has **entityTypes**, **excludeHeaderAttributes=true** and **valid entity attributes (not relationship) in 'attributes' field** , in the request payload.
**Request**: {
"excludeDeletedEntities": true,
"includeSubClassifications": true,
"includeSubTypes": true,
"includeClassificationAttributes": true,
"limit": 25,
"offset": 0,
"typeName": "hdfs_path",
"attributes": ["path", "name"],
"excludeHeaderAttributes": "true"
}
**Response**: {
"queryType": "BASIC",
"searchParameters": {
"typeName": "hdfs_path",
"excludeDeletedEntities": true,
"includeClassificationAttributes": true,
"includeSubTypes": true,
"includeSubClassifications": true,
"limit": 25,
"offset": 0,
"attributes": ["path", "name"]
},
"attributes": {
"name": ["path", "name"],
"values": [
["/data/warehouse/customer", "customer"],
["/data/warehouse/sales", "sales"]
]
},
"approximateCount": 2
}
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
Diff: https://reviews.apache.org/r/74109/diff/6/
Changes: https://reviews.apache.org/r/74109/diff/5-6/
Testing
-------
Unit testcases added
Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
Performance Readings:
Basic Search on 20K entities, limit 1000
Thanks,
Pinal Shah
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
> On Sept. 14, 2022, 11:11 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
> > Lines 478 (patched)
> > <https://reviews.apache.org/r/74109/diff/4/?file=2269974#file2269974line478>
> >
> > Use of HashSet here will eliminate duplicate rows. Is this by choice? Wouldn't this result in incorrect handling of pagination (offset)?
These elimination happens after handling pagination and getting the resultSet.
but yes, you are correct, from user end it will be confusing, that they have requested for 10, and have got less.
> On Sept. 14, 2022, 11:11 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
> > Lines 507 (patched)
> > <https://reviews.apache.org/r/74109/diff/4/?file=2269974#file2269974line507>
> >
> > return here would result in skipping of scrubSearchResults() - which is incorrect. Please review and update.
> >
> > Instead of introducing #471 - #509, consider updating existing for loop below at #549, like:
> >
> > for (AtlasVertex atlasVertex : resultList) {
> > if (searchContext.excludeHeaderAttributes()) {
> > // code from #475 - #506
> > } else {
> > // existing code
> > ...
> > }
> > }
Correct, but we are skipping many attributes, i.e classifications and also we don't have 'entities' (we just have the 'attributes') in AtlasSearchResult
I thought to align this with DSL search, Select Clause where AtlasSearchResult has only 'attributes' field.
If we add support to scrubSearchResult
- we need to create 'entities' from what information
- we need to get 'guid' from the vertex
- we need to get 'classifications'
- we need to add 'referredEntities'
getting classifications and referredEntities will reduce the performance which we are expecting with this improvement.
I will try implementing this once.
Any suugestions?
> On Sept. 14, 2022, 11:11 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
> > Line 1356 (original), 1356 (patched)
> > <https://reviews.apache.org/r/74109/diff/4/?file=2269976#file2269976line1356>
> >
> > Consider retaining getVertexAttribute() as private, based on comment in EntityDiscoveryService.java #494.
comment on #494,
By entityRetriever.getVertexAttribute();
do you mean entityRetriever.getVertexAttribute(vertex, attribute); ?
We dont have any existing method, other than this which returns the value of attribute.
And to access this in EntityDiscoveryService, we need to make this as public
- Pinal
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224671
-----------------------------------------------------------
On Sept. 14, 2022, 12:11 p.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2022, 12:11 p.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
> **Request**: {
> "excludeDeletedEntities": true,
> "includeSubClassifications": true,
> "includeSubTypes": true,
> "includeClassificationAttributes": true,
> "limit": 25,
> "offset": 0,
> "typeName": "hdfs_path",
> "attributes": ["path", "name"],
> "excludeHeaderAttributes": "true"
> }
>
> **Response**: {
> "queryType": "BASIC",
> "searchParameters": {
> "typeName": "hdfs_path",
> "excludeDeletedEntities": true,
> "includeClassificationAttributes": true,
> "includeSubTypes": true,
> "includeSubClassifications": true,
> "limit": 25,
> "offset": 0,
> "attributes": ["path", "name"]
> },
> "attributes": {
> "name": ["path", "name"],
> "values": [
> ["/data/warehouse/customer", "customer"],
> ["/data/warehouse/sales", "sales"]
> ]
> },
> "approximateCount": 2
> }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java a8fe5a762
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/5/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
> Performance Readings:
> Basic Search on 20K entities, limit 1000
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224671
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 478 (patched)
<https://reviews.apache.org/r/74109/#comment313464>
Use of HashSet here will eliminate duplicate rows. Is this by choice? Wouldn't this result in incorrect handling of pagination (offset)?
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 494 (patched)
<https://reviews.apache.org/r/74109/#comment313463>
Consider replacing #494 - #499 with
Object value = entityRetriever.getVertexAttribute(vertex, attribute);
i.e. use entityRetriever.getVertexAttribute() irrespective of the type of attribute - as it is generic enough to handle all types, including obj-ref/map/array/primitives.
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 507 (patched)
<https://reviews.apache.org/r/74109/#comment313465>
return here would result in skipping of scrubSearchResults() - which is incorrect. Please review and update.
Instead of introducing #471 - #509, consider updating existing for loop below at #549, like:
for (AtlasVertex atlasVertex : resultList) {
if (searchContext.excludeHeaderAttributes()) {
// code from #475 - #506
} else {
// existing code
...
}
}
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
Lines 356 (patched)
<https://reviews.apache.org/r/74109/#comment313466>
Validations in this if block seem to be valid irrespective of whether excludeHeaderAttributes is set or not. If yes, consider removing #356 and #370.
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
Line 1356 (original), 1356 (patched)
<https://reviews.apache.org/r/74109/#comment313467>
Consider retaining getVertexAttribute() as private, based on comment in EntityDiscoveryService.java #494.
- Madhan Neethiraj
On Sept. 14, 2022, 12:11 p.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2022, 12:11 p.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
> **Request**: {
> "excludeDeletedEntities": true,
> "includeSubClassifications": true,
> "includeSubTypes": true,
> "includeClassificationAttributes": true,
> "limit": 25,
> "offset": 0,
> "typeName": "hdfs_path",
> "attributes": ["path", "name"],
> "excludeHeaderAttributes": "true"
> }
>
> **Response**: {
> "queryType": "BASIC",
> "searchParameters": {
> "typeName": "hdfs_path",
> "excludeDeletedEntities": true,
> "includeClassificationAttributes": true,
> "includeSubTypes": true,
> "includeSubClassifications": true,
> "limit": 25,
> "offset": 0,
> "attributes": ["path", "name"]
> },
> "attributes": {
> "name": ["path", "name"],
> "values": [
> ["/data/warehouse/customer", "customer"],
> ["/data/warehouse/sales", "sales"]
> ]
> },
> "approximateCount": 2
> }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java a8fe5a762
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/4/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
> Performance Readings:
> Basic Search on 20K entities, limit 1000
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/
-----------------------------------------------------------
(Updated Sept. 14, 2022, 12:11 p.m.)
Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
Changes
-------
addressed review comments (supported referred attributes)
Bugs: ATLAS-4671
https://issues.apache.org/jira/browse/ATLAS-4671
Repository: atlas
Description
-------
**Issue** : Basic search has AtlasEntityHeader of each entity in the response.
AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
**Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
**Request**: {
"excludeDeletedEntities": true,
"includeSubClassifications": true,
"includeSubTypes": true,
"includeClassificationAttributes": true,
"limit": 25,
"offset": 0,
"typeName": "hdfs_path",
"attributes": ["path", "name"],
"excludeHeaderAttributes": "true"
}
**Response**: {
"queryType": "BASIC",
"searchParameters": {
"typeName": "hdfs_path",
"excludeDeletedEntities": true,
"includeClassificationAttributes": true,
"includeSubTypes": true,
"includeSubClassifications": true,
"limit": 25,
"offset": 0,
"attributes": ["path", "name"]
},
"attributes": {
"name": ["path", "name"],
"values": [
["/data/warehouse/customer", "customer"],
["/data/warehouse/sales", "sales"]
]
},
"approximateCount": 2
}
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java a8fe5a762
repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
Diff: https://reviews.apache.org/r/74109/diff/4/
Changes: https://reviews.apache.org/r/74109/diff/3-4/
Testing
-------
Unit testcases added
Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
Performance Readings:
Basic Search on 20K entities, limit 1000
Thanks,
Pinal Shah
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/
-----------------------------------------------------------
(Updated Sept. 14, 2022, 5:38 a.m.)
Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
Bugs: ATLAS-4671
https://issues.apache.org/jira/browse/ATLAS-4671
Repository: atlas
Description (updated)
-------
**Issue** : Basic search has AtlasEntityHeader of each entity in the response.
AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
**Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
**Request**: {
"excludeDeletedEntities": true,
"includeSubClassifications": true,
"includeSubTypes": true,
"includeClassificationAttributes": true,
"limit": 25,
"offset": 0,
"typeName": "hdfs_path",
"attributes": ["path", "name"],
"excludeHeaderAttributes": "true"
}
**Response**: {
"queryType": "BASIC",
"searchParameters": {
"typeName": "hdfs_path",
"excludeDeletedEntities": true,
"includeClassificationAttributes": true,
"includeSubTypes": true,
"includeSubClassifications": true,
"limit": 25,
"offset": 0,
"attributes": ["path", "name"]
},
"attributes": {
"name": ["path", "name"],
"values": [
["/data/warehouse/customer", "customer"],
["/data/warehouse/sales", "sales"]
]
},
"approximateCount": 2
}
Diffs
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
Diff: https://reviews.apache.org/r/74109/diff/3/
Testing (updated)
-------
Unit testcases added
Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
Performance Readings:
Basic Search on 20K entities, limit 1000
Thanks,
Pinal Shah
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Sept. 13, 2022, 4:44 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
> > Lines 373 (patched)
> > <https://reviews.apache.org/r/74109/diff/3/?file=2269964#file2269964line373>
> >
> > It is not clear why entity-ref attributes are not allowed when excludeHeaderAttributes is true. Is it for faster query response i.e., to avoid look up? Given the attribute is explicitly asked for, the caller should know the performance impact. Why not allow this case?
>
> Pinal Shah wrote:
> Whenever "excludeHeaderAttributes":"true", response is bit different
>
> Example:
> request: "attributes":["qualifiedName"], "excludeHeaderAttributes":"true"
> response: {
> "entities": [],
> "attributes": {
> "name": ["qualifiedName"],
> "values": [
> ["sales@sys"],
> ["customer@sys"]
> ]
> }
> }
>
> List<AtlasEntityHeader> entities, will be blank
> AttributeSearchResult attributes, populate attribute values here.
>
>
> Now case when
> request: "attributes":["qualifiedName","db"], "excludeHeaderAttributes":"true"
> We need to fit below "db" value json, to AttributeSearchResult
> "db": {
> "guid": "6e44a899-320c-4026-a66a-d67a76ef7c41",
> "typeName": "hive_db",
> "uniqueAttributes": {
> "qualifiedName": "sys@cl1"
> }
> }
AtlasSearchResult.values is of type List<List<Object>>, hence should be able to handle obj-ref values as well - right?
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224662
-----------------------------------------------------------
On Sept. 14, 2022, 5:38 a.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2022, 5:38 a.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
> **Request**: {
> "excludeDeletedEntities": true,
> "includeSubClassifications": true,
> "includeSubTypes": true,
> "includeClassificationAttributes": true,
> "limit": 25,
> "offset": 0,
> "typeName": "hdfs_path",
> "attributes": ["path", "name"],
> "excludeHeaderAttributes": "true"
> }
>
> **Response**: {
> "queryType": "BASIC",
> "searchParameters": {
> "typeName": "hdfs_path",
> "excludeDeletedEntities": true,
> "includeClassificationAttributes": true,
> "includeSubTypes": true,
> "includeSubClassifications": true,
> "limit": 25,
> "offset": 0,
> "attributes": ["path", "name"]
> },
> "attributes": {
> "name": ["path", "name"],
> "values": [
> ["/data/warehouse/customer", "customer"],
> ["/data/warehouse/sales", "sales"]
> ]
> },
> "approximateCount": 2
> }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/3/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
> Performance Readings:
> Basic Search on 20K entities, limit 1000
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
> On Sept. 13, 2022, 4:44 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
> > Lines 373 (patched)
> > <https://reviews.apache.org/r/74109/diff/3/?file=2269964#file2269964line373>
> >
> > It is not clear why entity-ref attributes are not allowed when excludeHeaderAttributes is true. Is it for faster query response i.e., to avoid look up? Given the attribute is explicitly asked for, the caller should know the performance impact. Why not allow this case?
Whenever "excludeHeaderAttributes":"true", response is bit different
Example:
request: "attributes":["qualifiedName"], "excludeHeaderAttributes":"true"
response: {
"entities": [],
"attributes": {
"name": ["qualifiedName"],
"values": [
["sales@sys"],
["customer@sys"]
]
}
}
List<AtlasEntityHeader> entities, will be blank
AttributeSearchResult attributes, populate attribute values here.
Now case when
request: "attributes":["qualifiedName","db"], "excludeHeaderAttributes":"true"
We need to fit below "db" value json, to AttributeSearchResult
"db": {
"guid": "6e44a899-320c-4026-a66a-d67a76ef7c41",
"typeName": "hive_db",
"uniqueAttributes": {
"qualifiedName": "sys@cl1"
}
}
- Pinal
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224662
-----------------------------------------------------------
On Sept. 13, 2022, 1:46 p.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2022, 1:46 p.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph to get all the header attributes.
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/3/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
> On Sept. 13, 2022, 4:44 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
> > Lines 373 (patched)
> > <https://reviews.apache.org/r/74109/diff/3/?file=2269964#file2269964line373>
> >
> > It is not clear why entity-ref attributes are not allowed when excludeHeaderAttributes is true. Is it for faster query response i.e., to avoid look up? Given the attribute is explicitly asked for, the caller should know the performance impact. Why not allow this case?
>
> Pinal Shah wrote:
> Whenever "excludeHeaderAttributes":"true", response is bit different
>
> Example:
> request: "attributes":["qualifiedName"], "excludeHeaderAttributes":"true"
> response: {
> "entities": [],
> "attributes": {
> "name": ["qualifiedName"],
> "values": [
> ["sales@sys"],
> ["customer@sys"]
> ]
> }
> }
>
> List<AtlasEntityHeader> entities, will be blank
> AttributeSearchResult attributes, populate attribute values here.
>
>
> Now case when
> request: "attributes":["qualifiedName","db"], "excludeHeaderAttributes":"true"
> We need to fit below "db" value json, to AttributeSearchResult
> "db": {
> "guid": "6e44a899-320c-4026-a66a-d67a76ef7c41",
> "typeName": "hive_db",
> "uniqueAttributes": {
> "qualifiedName": "sys@cl1"
> }
> }
>
> Madhan Neethiraj wrote:
> AtlasSearchResult.values is of type List<List<Object>>, hence should be able to handle obj-ref values as well - right?
yes Madhan
Thanks for reviewing.i will add this support of referred attributes
- Pinal
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224662
-----------------------------------------------------------
On Sept. 14, 2022, 5:38 a.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2022, 5:38 a.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
> **Request**: {
> "excludeDeletedEntities": true,
> "includeSubClassifications": true,
> "includeSubTypes": true,
> "includeClassificationAttributes": true,
> "limit": 25,
> "offset": 0,
> "typeName": "hdfs_path",
> "attributes": ["path", "name"],
> "excludeHeaderAttributes": "true"
> }
>
> **Response**: {
> "queryType": "BASIC",
> "searchParameters": {
> "typeName": "hdfs_path",
> "excludeDeletedEntities": true,
> "includeClassificationAttributes": true,
> "includeSubTypes": true,
> "includeSubClassifications": true,
> "limit": 25,
> "offset": 0,
> "attributes": ["path", "name"]
> },
> "attributes": {
> "name": ["path", "name"],
> "values": [
> ["/data/warehouse/customer", "customer"],
> ["/data/warehouse/sales", "sales"]
> ]
> },
> "approximateCount": 2
> }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/3/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
> Performance Readings:
> Basic Search on 20K entities, limit 1000
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/#review224662
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
Lines 179 (patched)
<https://reviews.apache.org/r/74109/#comment313457>
"Referenced Attribute" => "Entity-reference attribute"
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
Lines 357 (patched)
<https://reviews.apache.org/r/74109/#comment313458>
Consider moving #153 within validateAttributes(), to keep the context (in which validateAttributes is called) together:
private void validateAttributes() {
if (CollectionUtils.isNotEmpty(entityTypes) && CollectionUtils.isNotEmpty(searchParameters.getAttributes())) {
...
if (searchParameters.getExcludeHeaderAttributes() &&
(attributeType instanceof AtlasEntityType || attributeType instanceof AtlasBuiltInTypes.AtlasObjectIdType)) {
throw new AtlasBaseException(AtlasErrorCode.REFERENCED_ATTRIBUTE_NOT_ALLOWED, attr, String.valueOf(searchParameters.getExcludeHeaderAttributes()));
}
...
}
}
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
Lines 373 (patched)
<https://reviews.apache.org/r/74109/#comment313459>
It is not clear why entity-ref attributes are not allowed when excludeHeaderAttributes is true. Is it for faster query response i.e., to avoid look up? Given the attribute is explicitly asked for, the caller should know the performance impact. Why not allow this case?
- Madhan Neethiraj
On Sept. 13, 2022, 1:46 p.m., Pinal Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74109/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2022, 1:46 p.m.)
>
>
> Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
>
>
> Bugs: ATLAS-4671
> https://issues.apache.org/jira/browse/ATLAS-4671
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Issue** : Basic search has AtlasEntityHeader of each entity in the response.
> AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph to get all the header attributes.
>
> **Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
> intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
> intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
> repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
> repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
>
>
> Diff: https://reviews.apache.org/r/74109/diff/3/
>
>
> Testing
> -------
>
> Unit testcases added
> Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
>
>
> Thanks,
>
> Pinal Shah
>
>
Re: Review Request 74109: ATLAS-4671 : Basic Search : Exclude Header attributes of entities from the response
Posted by Pinal Shah <pi...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74109/
-----------------------------------------------------------
(Updated Sept. 13, 2022, 1:46 p.m.)
Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra.
Bugs: ATLAS-4671
https://issues.apache.org/jira/browse/ATLAS-4671
Repository: atlas
Description
-------
**Issue** : Basic search has AtlasEntityHeader of each entity in the response.
AtlasEntityHeader has many attributes including classification and terms. hence for each attribute, it will request janusgraph to get all the header attributes.
**Approach** : To overcome, we can add a flag to exclude other attributes and add only selected attributes from 'attributes' field in the response
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 608342433
intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java 79f5aae0d
intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 78fb4a48f
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 8fbc22fa0
repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 01954d07e
repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java fbc739652
Diff: https://reviews.apache.org/r/74109/diff/3/
Changes: https://reviews.apache.org/r/74109/diff/2-3/
Testing
-------
Unit testcases added
Precommit : https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/
Thanks,
Pinal Shah