You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Apoorv Naik <na...@gmail.com> on 2017/07/17 22:31:19 UTC

Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

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

Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.


Repository: atlas


Description
-------

Initial draft for multi-vertex graph query in atlas


Diffs
-----

  graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
  graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
  graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
  graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
  graphdb/titan0/pom.xml ceda1bb2 
  graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
  graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
  graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
  graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
  repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 


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


Testing
-------


Thanks,

Apoorv Naik

Re: Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

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




graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java
Lines 29 (patched)
<https://reviews.apache.org/r/60927/#comment255998>

    This interface doesn't seem to be used. Please review. If not required, please remove.


- Madhan Neethiraj


On July 17, 2017, 10:31 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60927/
> -----------------------------------------------------------
> 
> (Updated July 17, 2017, 10:31 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Initial draft for multi-vertex graph query in atlas
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
>   graphdb/titan0/pom.xml ceda1bb2 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 
> 
> 
> Diff: https://reviews.apache.org/r/60927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

Posted by Apoorv Naik <na...@gmail.com>.

> On July 19, 2017, 8:46 a.m., Graham Wallis wrote:
> > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java
> > Lines 29 (patched)
> > <https://reviews.apache.org/r/60927/diff/1/?file=1778021#file1778021line29>
> >
> >     Minor point but it would be nicer to name this parameter 'vertices' for consistency with other changes.

Will do.


> On July 19, 2017, 8:46 a.m., Graham Wallis wrote:
> > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java
> > Lines 91 (patched)
> > <https://reviews.apache.org/r/60927/diff/1/?file=1778026#file1778026line91>
> >
> >     Trailing whitespace.

Will remove


> On July 19, 2017, 8:46 a.m., Graham Wallis wrote:
> > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java
> > Lines 80 (patched)
> > <https://reviews.apache.org/r/60927/diff/1/?file=1778028#file1778028line80>
> >
> >     I'm slightly worried because the multiQuery() method appears to be deprecated (in Titan 1.0.0). Presumably that suggests there is a better way to do this with Titan, although I'm not sure what. I suspect we need to construct the MVQ explicitly rather than pass the vertex or Collection to multiQuery().

I looked around for any updated document around the deprecation of this method but couldn't find one. Let's explore other options to ensure that we use the most up to date API.


> On July 19, 2017, 8:46 a.m., Graham Wallis wrote:
> > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java
> > Lines 396 (patched)
> > <https://reviews.apache.org/r/60927/diff/1/?file=1778028#file1778028line396>
> >
> >     Should this be accepting (and transforming) an edges parameter?

Good catch. I forgot to rename the parameter.


- Apoorv


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


On July 17, 2017, 10:31 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60927/
> -----------------------------------------------------------
> 
> (Updated July 17, 2017, 10:31 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Initial draft for multi-vertex graph query in atlas
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
>   graphdb/titan0/pom.xml ceda1bb2 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 
> 
> 
> Diff: https://reviews.apache.org/r/60927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60927/#review180908
-----------------------------------------------------------




graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java
Lines 29 (patched)
<https://reviews.apache.org/r/60927/#comment256251>

    Minor point but it would be nicer to name this parameter 'vertices' for consistency with other changes.



graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java
Lines 91 (patched)
<https://reviews.apache.org/r/60927/#comment256252>

    Trailing whitespace.



graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java
Lines 80 (patched)
<https://reviews.apache.org/r/60927/#comment256250>

    I'm slightly worried because the multiQuery() method appears to be deprecated (in Titan 1.0.0). Presumably that suggests there is a better way to do this with Titan, although I'm not sure what. I suspect we need to construct the MVQ explicitly rather than pass the vertex or Collection to multiQuery().



graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java
Lines 396 (patched)
<https://reviews.apache.org/r/60927/#comment256253>

    Should this be accepting (and transforming) an edges parameter?


- Graham Wallis


On July 17, 2017, 10:31 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60927/
> -----------------------------------------------------------
> 
> (Updated July 17, 2017, 10:31 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Initial draft for multi-vertex graph query in atlas
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
>   graphdb/titan0/pom.xml ceda1bb2 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 
> 
> 
> Diff: https://reviews.apache.org/r/60927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

Posted by Apoorv Naik <na...@gmail.com>.

> On July 19, 2017, 10:54 a.m., David Radley wrote:
> > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java
> > Line 147 (original), 147 (patched)
> > <https://reviews.apache.org/r/60927/diff/1/?file=1778020#file1778020line147>
> >
> >     I am unsure about the intended scope of this fix, I suggest that you introduce unit tests at this stage to test all the represtative queries that this fix will enable and error cases.

Will definitely add tests after I test out the usecases that it might solve.


- Apoorv


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


On July 17, 2017, 10:31 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60927/
> -----------------------------------------------------------
> 
> (Updated July 17, 2017, 10:31 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Initial draft for multi-vertex graph query in atlas
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
>   graphdb/titan0/pom.xml ceda1bb2 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 
> 
> 
> Diff: https://reviews.apache.org/r/60927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

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




graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java
Line 147 (original), 147 (patched)
<https://reviews.apache.org/r/60927/#comment256256>

    I am unsure about the intended scope of this fix, I suggest that you introduce unit tests at this stage to test all the represtative queries that this fix will enable and error cases.


- David Radley


On July 17, 2017, 10:31 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60927/
> -----------------------------------------------------------
> 
> (Updated July 17, 2017, 10:31 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Initial draft for multi-vertex graph query in atlas
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
>   graphdb/titan0/pom.xml ceda1bb2 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 
> 
> 
> Diff: https://reviews.apache.org/r/60927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 60927: ATLAS-1957: Multi-vertex atlas graph query

Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60927/#review180754
-----------------------------------------------------------




repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java
Lines 184 (patched)
<https://reviews.apache.org/r/60927/#comment255971>

    TODO: The multi-vertex query will be used here


- Apoorv Naik


On July 17, 2017, 10:31 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60927/
> -----------------------------------------------------------
> 
> (Updated July 17, 2017, 10:31 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Initial draft for multi-vertex graph query in atlas
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java dded76f8 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanMultiVertexQuery.java PRE-CREATION 
>   graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java ac7ff9e8 
>   graphdb/titan0/pom.xml ceda1bb2 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java 2408287b 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0AtlasMultiVertexQuery.java PRE-CREATION 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java ffb6b37f 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1AtlasMultiVertexQuery.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2 
> 
> 
> Diff: https://reviews.apache.org/r/60927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>