> On Aug. 9, 2017, 10:53 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
> > Lines 61 (patched)
> > <https://reviews.apache.org/r/61502/diff/1/?file=1792991#file1792991line61>
> >
> >     I am not sure Apoorv is quite right. The code seems to be saying if 
> > there is a supplied type then it needs to exist. If there is a supplied 
> > classification it needs to exist. Is it valid to not supply a 
> > classification type or an entity type? Should we test for this as an error 
> > case?   
> >     
> >     I also suggest we do not put text as a message insert . The text should 
> > be in the message template and have unique message numbers. Ideally the 
> > message should put out the search request that is being issued - so the 
> > user has the context of what type is missing / invalid.

Yes the code does exactly what you stated above. It's a valid use-case to not 
supply typename or the classification, the search request MUST contain atleast 
one of the following 

1. TypeName
2. Classification
3. FreeText (lucene style query text)

It can also contain any combination of the above parameters. Does that clarify 
your concern ?

Thanks for catching the error code mistake, I've updated the code to reflect 
the changes. One thing that's not doable right now is adding details regarding 
the error, ideally we'd want the error JSON to have a details field which can 
capture more information if needed.


- Apoorv


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


On Aug. 9, 2017, 5:26 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61502/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2017, 5:26 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2025
>     https://issues.apache.org/jira/browse/ATLAS-2025
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Right now there's no such validation and as a (unwanted) side-effect the 
> following happens
> 
> 1. Internal error when type name is invalid (NPE)
> 2. Type is valid but classification is invalid - the result contains entities 
> which don't have the tag (exactly opposite behavior)
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6 
>   
> repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
>  66dd7484 
>   repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 
> 929f8d00 
> 
> 
> Diff: https://reviews.apache.org/r/61502/diff/2/
> 
> 
> Testing
> -------
> 
> Validated the correctness of results using curl/postman calls, any invalid 
> type name / classification throws 400 now.
> 
> mvn clean package -Pdist,berkeley-elasticsearch executes successfully
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Reply via email to