atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apoorv Naik <naik.apo...@gmail.com>
Subject Re: Review Request 61502: ATLAS-2025: Validation on type name and classification
Date Wed, 09 Aug 2017 17:26:33 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message