You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Sarath Subramanian <sa...@apache.org> on 2017/04/29 00:36:06 UTC

Review Request 58868: [ATLAS-1761] : Improve attribute search to enable search based on display text and guid

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58868/
-----------------------------------------------------------

Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and Suma Shivaprasad.


Bugs: ATLAS-1761
    https://issues.apache.org/jira/browse/ATLAS-1761


Repository: atlas


Description
-------

Improve attribute search to enable search based on display text and guid


Diffs
-----

  repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 07190dbf 
  repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java d3413c23 
  repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 633fad03 
  webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 76ad88cb 


Diff: https://reviews.apache.org/r/58868/diff/1/


Testing
-------

Tsted using POSTMAN client.


Thanks,

Sarath Subramanian


Re: Review Request 58868: [ATLAS-1761] : Improve attribute search to enable search based on display text and guid

Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58868/#review173570
-----------------------------------------------------------




repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Line 217 (original), 220 (patched)
<https://reviews.apache.org/r/58868/#comment246563>

    Incorrect formatting



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 227 (patched)
<https://reviews.apache.org/r/58868/#comment246564>

    Does this loop only interate once? If not it will pick up the last attribute name - is this the required behaviour?
    
    If we pick up the last ofa list of default attribute names - then is there an issue that any attribute value associated with this attribute name might mismatch it.



webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java
Line 202 (original), 202 (patched)
<https://reviews.apache.org/r/58868/#comment246565>

    This change of logic implies an API behvaviour change - I assume we need to document this. Does it effect basic search and DSL or just DSL ?



webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java
Line 203 (original), 203 (patched)
<https://reviews.apache.org/r/58868/#comment246566>

    I think all of the changes in this patch need unit tests.


- David Radley


On April 29, 2017, 8:28 p.m., Sarath Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58868/
> -----------------------------------------------------------
> 
> (Updated April 29, 2017, 8:28 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1761
>     https://issues.apache.org/jira/browse/ATLAS-1761
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Improve attribute search to enable search based on display text and guid
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java f3f6ee22 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java d3413c23 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 633fad03 
>   webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 81dbe25a 
> 
> 
> Diff: https://reviews.apache.org/r/58868/diff/2/
> 
> 
> Testing
> -------
> 
> Tsted using POSTMAN client.
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>


Re: Review Request 58868: [ATLAS-1761] : Improve attribute search to enable search based on display text and guid

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58868/#review173433
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On April 29, 2017, 8:28 p.m., Sarath Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58868/
> -----------------------------------------------------------
> 
> (Updated April 29, 2017, 8:28 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1761
>     https://issues.apache.org/jira/browse/ATLAS-1761
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Improve attribute search to enable search based on display text and guid
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java f3f6ee22 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java d3413c23 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 633fad03 
>   webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 81dbe25a 
> 
> 
> Diff: https://reviews.apache.org/r/58868/diff/2/
> 
> 
> Testing
> -------
> 
> Tsted using POSTMAN client.
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>


Re: Review Request 58868: [ATLAS-1761] : Improve attribute search to enable search based on display text and guid

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58868/#review173459
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On April 29, 2017, 8:28 p.m., Sarath Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58868/
> -----------------------------------------------------------
> 
> (Updated April 29, 2017, 8:28 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1761
>     https://issues.apache.org/jira/browse/ATLAS-1761
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Improve attribute search to enable search based on display text and guid
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java f3f6ee22 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java d3413c23 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 633fad03 
>   webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 81dbe25a 
> 
> 
> Diff: https://reviews.apache.org/r/58868/diff/2/
> 
> 
> Testing
> -------
> 
> Tsted using POSTMAN client.
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>


Re: Review Request 58868: [ATLAS-1761] : Improve attribute search to enable search based on display text and guid

Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58868/
-----------------------------------------------------------

(Updated April 29, 2017, 1:28 p.m.)


Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and Suma Shivaprasad.


Changes
-------

addressed review comments.


Bugs: ATLAS-1761
    https://issues.apache.org/jira/browse/ATLAS-1761


Repository: atlas


Description
-------

Improve attribute search to enable search based on display text and guid


Diffs (updated)
-----

  repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java f3f6ee22 
  repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java d3413c23 
  repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 633fad03 
  webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 81dbe25a 


Diff: https://reviews.apache.org/r/58868/diff/2/

Changes: https://reviews.apache.org/r/58868/diff/1-2/


Testing
-------

Tsted using POSTMAN client.


Thanks,

Sarath Subramanian


Re: Review Request 58868: [ATLAS-1761] : Improve attribute search to enable search based on display text and guid

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58868/#review173412
-----------------------------------------------------------




repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Line 212 (original), 210 (patched)
<https://reviews.apache.org/r/58868/#comment246420>

    when attrName is specified and if no attribute is found, then this should be flagged as an error condition, just as in earlier implementation - with:
    
        if (attribute == null) {
            throw new AtlasBaseException(AtlasErrorCode.UNKNOWN_ATTRIBUTE, attrName, typeName);
        }
        
        
    With the current change, this condition will assume search by guid - like #222



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 234 (patched)
<https://reviews.apache.org/r/58868/#comment246421>

    Consider moving lines #234 - #236 in the else block above at #228.



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 274 (patched)
<https://reviews.apache.org/r/58868/#comment246418>

    This debug log will be printed for each entry in the search result. Perphas you mean this to be outside the while() loop?


- Madhan Neethiraj


On April 29, 2017, 12:36 a.m., Sarath Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58868/
> -----------------------------------------------------------
> 
> (Updated April 29, 2017, 12:36 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1761
>     https://issues.apache.org/jira/browse/ATLAS-1761
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Improve attribute search to enable search based on display text and guid
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 07190dbf 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java d3413c23 
>   repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java 633fad03 
>   webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 76ad88cb 
> 
> 
> Diff: https://reviews.apache.org/r/58868/diff/1/
> 
> 
> Testing
> -------
> 
> Tsted using POSTMAN client.
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>