Kam

REST work merge to OryxProject:master
I am ok with not using an ExceptionMapper which would be injected when an
exception is thrown (its just standard JAX-RS way of handling exceptions).
But whether we return a 404/400 we shouldn't be hardcoding the int value
and using the Enum.


On Fri, Aug 8, 2014 at 5:59 AM, Sean Owen <notifications@github.com> wrote:

> In
> oryx-ml-oryx-serving/src/main/java/com/cloudera/oryx/ml/serving/als/RecommendToAnonymous.java:
>
> > @@ -48,6 +51,12 @@
> >  public final class RecommendToAnonymous {
> >
> >    @GET
> > +  @Produces(MediaType.APPLICATION_JSON)
> > +  public Response get() {
> > +    return Response.status(400).entity(new ErrorResponse(400, "one or more itemIDs required")).build();
>
> This PR is already committed but you can open new ones. I'm split on
> whether it's clearer to just return status X, or throw an exception that is
> mapped elsewhere to X. The former seems cleaner, especially since this
> probably means making a new "BadRequestException" to clearly select the
> status code anyway. I still think it would be fine to let such a request
> return 404 too, and remove this method.
> … <#147b50f13d625a67_>
> On Fri, Aug 8, 2014 at 7:22 AM, Suneel Marthi <notifications@github.com>
> wrote: In
> oryx-ml-oryx-serving/src/main/java/com/cloudera/oryx/ml/serving/als/RecommendToAnonymous.java:
> > @@ -48,6 +51,12 @@ > public final class RecommendToAnonymous { > > @GET
> <https://github.com/GET> > + @Produces(MediaType.APPLICATION_JSON) > +
> public Response get() { > + return Response.status(400).entity(new
> ErrorResponse(400, "one or more itemIDs required")).build(); Replace '400'
> by Response.Status.BAD_REQUEST. This should be thrown as an Exception (via
> ExceptionMapper). Replace '400' in ErrorResponse bt
> Response.Status.BAD_REQUEST.getStatusCode() — Reply to this email directly
> or view it on GitHub <
> https://github.com/OryxProject/oryx/pull/7/files#r15979484>.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/OryxProject/oryx/pull/7/files#r15985589>.
>