You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Tom Beerbower <tb...@hortonworks.com> on 2014/10/06 23:55:12 UTC

Review Request 26386: Views : View API request for non-existent view should return 404

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

Review request for Ambari, Alejandro Fernandez and Nate Cole.


Bugs: AMBARI-7664
    https://issues.apache.org/jira/browse/AMBARI-7664


Repository: ambari


Description
-------

The request ...

{code}
GET http://c6401.ambari.apache.org:8080/api/v1/views/BadView
{code}

... where 'BadView' is not a deployed view, should return a 404.

{code}
{
  "status" : 404,
  "message" : "The requested resource doesn't exist: View not found, ViewInfo/view_name=BadView"
}
{code}


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 48767d5 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 1d0eef4 

Diff: https://reviews.apache.org/r/26386/diff/


Testing
-------

Manual tests.

New unit test added.  All existing tests pass ...


Results :

Tests run: 2077, Failures: 0, Errors: 0, Skipped: 16
...
[INFO] Executed tasks
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 31:31.433s
[INFO] Finished at: Mon Oct 06 17:41:47 EDT 2014
[INFO] Final Memory: 38M/476M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower


Re: Review Request 26386: Views : View API request for non-existent view should return 404

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26386/#review55586
-----------------------------------------------------------

Ship it!


Ship It!

- Alejandro Fernandez


On Oct. 6, 2014, 9:55 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26386/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2014, 9:55 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-7664
>     https://issues.apache.org/jira/browse/AMBARI-7664
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The request ...
> 
> {code}
> GET http://c6401.ambari.apache.org:8080/api/v1/views/BadView
> {code}
> 
> ... where 'BadView' is not a deployed view, should return a 404.
> 
> {code}
> {
>   "status" : 404,
>   "message" : "The requested resource doesn't exist: View not found, ViewInfo/view_name=BadView"
> }
> {code}
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 48767d5 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 1d0eef4 
> 
> Diff: https://reviews.apache.org/r/26386/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit test added.  All existing tests pass ...
> 
> 
> Results :
> 
> Tests run: 2077, Failures: 0, Errors: 0, Skipped: 16
> ...
> [INFO] Executed tasks
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 31:31.433s
> [INFO] Finished at: Mon Oct 06 17:41:47 EDT 2014
> [INFO] Final Memory: 38M/476M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 26386: Views : View API request for non-existent view should return 404

Posted by Tom Beerbower <tb...@hortonworks.com>.

> On Oct. 6, 2014, 10:56 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java, lines 137-140
> > <https://reviews.apache.org/r/26386/diff/1/?file=714276#file714276line137>
> >
> >     What about the case, say, you ask for all datanodes that are stopped, and all of them are running.  The predicate (I hope) won't be null, but the list will be empty.  Should not result in a 404.  If that is the case, then maybe this should be thrown at a lower layer?

Good question.  That is actually handled by the ReadHandler wich calls the ClusterController ...

    } catch (NoSuchResourceException e) {
      if (p == null) {
        // no predicate specified, resource requested by id
        result = new ResultImpl(new ResultStatus(ResultStatus.STATUS.NOT_FOUND, e.getMessage()));
      } else {
        // resource(s) requested using predicate
        result = new ResultImpl(new ResultStatus(ResultStatus.STATUS.OK, e));
        result.getResultTree().setProperty("isCollection", "true");
      }
    }
    
In the case that you mention, the predicate (p) will not be null so when the exception is caught the result status will be set to OK.  

In the case where the request is 'http://c6401.ambari.apache.org:8080/api/v1/views/BadView', the ReadHandler will get a null predicate but will create and pass a new predicate 'view_name = BadView' to the ClusterController.  The CC will throw the exception if the result set is null and the RH will handle it by setting the result status to NOT_FOUND (404).

Thanks!


- Tom


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


On Oct. 6, 2014, 10:55 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26386/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2014, 10:55 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-7664
>     https://issues.apache.org/jira/browse/AMBARI-7664
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The request ...
> 
> {code}
> GET http://c6401.ambari.apache.org:8080/api/v1/views/BadView
> {code}
> 
> ... where 'BadView' is not a deployed view, should return a 404.
> 
> {code}
> {
>   "status" : 404,
>   "message" : "The requested resource doesn't exist: View not found, ViewInfo/view_name=BadView"
> }
> {code}
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 48767d5 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 1d0eef4 
> 
> Diff: https://reviews.apache.org/r/26386/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit test added.  All existing tests pass ...
> 
> 
> Results :
> 
> Tests run: 2077, Failures: 0, Errors: 0, Skipped: 16
> ...
> [INFO] Executed tasks
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 31:31.433s
> [INFO] Finished at: Mon Oct 06 17:41:47 EDT 2014
> [INFO] Final Memory: 38M/476M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 26386: Views : View API request for non-existent view should return 404

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26386/#review55594
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java
<https://reviews.apache.org/r/26386/#comment95962>

    What about the case, say, you ask for all datanodes that are stopped, and all of them are running.  The predicate (I hope) won't be null, but the list will be empty.  Should not result in a 404.  If that is the case, then maybe this should be thrown at a lower layer?


- Nate Cole


On Oct. 6, 2014, 6:55 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26386/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2014, 6:55 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-7664
>     https://issues.apache.org/jira/browse/AMBARI-7664
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The request ...
> 
> {code}
> GET http://c6401.ambari.apache.org:8080/api/v1/views/BadView
> {code}
> 
> ... where 'BadView' is not a deployed view, should return a 404.
> 
> {code}
> {
>   "status" : 404,
>   "message" : "The requested resource doesn't exist: View not found, ViewInfo/view_name=BadView"
> }
> {code}
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 48767d5 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 1d0eef4 
> 
> Diff: https://reviews.apache.org/r/26386/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit test added.  All existing tests pass ...
> 
> 
> Results :
> 
> Tests run: 2077, Failures: 0, Errors: 0, Skipped: 16
> ...
> [INFO] Executed tasks
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 31:31.433s
> [INFO] Finished at: Mon Oct 06 17:41:47 EDT 2014
> [INFO] Final Memory: 38M/476M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 26386: Views : View API request for non-existent view should return 404

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26386/#review55600
-----------------------------------------------------------

Ship it!


Ship It!

- Nate Cole


On Oct. 6, 2014, 6:55 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26386/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2014, 6:55 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Nate Cole.
> 
> 
> Bugs: AMBARI-7664
>     https://issues.apache.org/jira/browse/AMBARI-7664
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The request ...
> 
> {code}
> GET http://c6401.ambari.apache.org:8080/api/v1/views/BadView
> {code}
> 
> ... where 'BadView' is not a deployed view, should return a 404.
> 
> {code}
> {
>   "status" : 404,
>   "message" : "The requested resource doesn't exist: View not found, ViewInfo/view_name=BadView"
> }
> {code}
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 48767d5 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 1d0eef4 
> 
> Diff: https://reviews.apache.org/r/26386/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit test added.  All existing tests pass ...
> 
> 
> Results :
> 
> Tests run: 2077, Failures: 0, Errors: 0, Skipped: 16
> ...
> [INFO] Executed tasks
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 31:31.433s
> [INFO] Finished at: Mon Oct 06 17:41:47 EDT 2014
> [INFO] Final Memory: 38M/476M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 26386: Views : View API request for non-existent view should return 404

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26386/
-----------------------------------------------------------

(Updated Oct. 6, 2014, 10:55 p.m.)


Review request for Ambari, Alejandro Fernandez and Nate Cole.


Changes
-------

update ClusterControllerImpl to check for null resource set.


Bugs: AMBARI-7664
    https://issues.apache.org/jira/browse/AMBARI-7664


Repository: ambari


Description
-------

The request ...

{code}
GET http://c6401.ambari.apache.org:8080/api/v1/views/BadView
{code}

... where 'BadView' is not a deployed view, should return a 404.

{code}
{
  "status" : 404,
  "message" : "The requested resource doesn't exist: View not found, ViewInfo/view_name=BadView"
}
{code}


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java 48767d5 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java 1d0eef4 

Diff: https://reviews.apache.org/r/26386/diff/


Testing
-------

Manual tests.

New unit test added.  All existing tests pass ...


Results :

Tests run: 2077, Failures: 0, Errors: 0, Skipped: 16
...
[INFO] Executed tasks
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 31:31.433s
[INFO] Finished at: Mon Oct 06 17:41:47 EDT 2014
[INFO] Final Memory: 38M/476M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower