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 58833: Typedef validations during create/update
Date Mon, 01 May 2017 20:21:23 GMT


> On April 28, 2017, 9:47 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
> > Lines 894 (patched)
> > <https://reviews.apache.org/r/58833/diff/1/?file=1702343#file1702343line896>
> >
> >     I would think there are places that we would want to send a not found . for
example if we try to update an entry but we cannot find it (i.e. the guid or name sent in
cannot be found). This logic seems to changing all the codes over. 
> >     
> >     It does not seem right to change the eror code like this - wouldn't it be better
to set the code correctly when it is originally created - rather than fix it up later.
> 
> Apoorv Naik wrote:
>     As per my experience with REST APIs I never saw a 404 during a PUT call always a
400 because that signals the user/caller that there's something wrong with the request and
the error message will provide more context on that. 
>     
>     404 -> 400 is only done for the update calls, GETs/DELETEs will still respond
with 404 in such scenario

The fixing of error code upfront is definitely the correct way to do it but the way atlas
code is structured by the time we throw the exception the context is lost so the code fragment
can't distinguish if the type being updated is invalid or any other typename is invalid. The
ideal fix would be to first do a get, then update the definition and commit that change to
the store. I think it's a bigger fix if we intend to go that route.


- Apoorv


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


On May 1, 2017, 8:17 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58833/
> -----------------------------------------------------------
> 
> (Updated May 1, 2017, 8:17 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1762
>     https://issues.apache.org/jira/browse/ATLAS-1762
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Following tests
> 
> 1. Creating a type with super type that doesn’t exist
> 2. Updating a type with super type that doesn’t exist
> 3. Creating a type with attribute of unknown data type
> 
> throw 404 Not Found error. Expected is 400 Bad Request.
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
82465bfc 
> 
> 
> Diff: https://reviews.apache.org/r/58833/diff/2/
> 
> 
> Testing
> -------
> 
> mvn clean package executes successfully
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>


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