Improve json_response to allow adjusting the HTTP code.

Improve json_response to allow adjusting the HTTP code.

Conservatively utilizes the new json_response behavior. For apis which claim
to respond with JSON, we should always respond with JSON. However, I know that
some of the non-JSON responses may be expected by clients, so I don't correct
everything that I could correct.

R=maruel@chromium.org
BUG=

[iannucci, 2013-10-27 11:46:48]

PTAL

[M-A, 2013-10-28 15:37:23]

Vadim, what do you think?

[vadimsh, 2013-10-28 17:56:35]

On 2013/10/28 15:37:23, M-A wrote:
> Vadim, what do you think?

I think it looks a little bit like a hack. But I don't have any solid
alternatives. For 404 and alike exceptions might be more appropriate. For OK-ish
codes exceptions won't work. Maybe something along the lines:

@json_response
def blah(...):
  if stuff:
    return {'msg': 'this would be code 200'}
  else:
    return JsonWithStatus({'msg': 'this is whatever'}, code=300)

(I'm a fan of strict typing whenever possible)

But it looks ugly...

[M-A, 2013-10-29 01:28:14]

On 2013/10/28 17:56:35, vadimsh wrote:
> On 2013/10/28 15:37:23, M-A wrote:
> > Vadim, what do you think?
> 
> I think it looks a little bit like a hack. But I don't have any solid
> alternatives. For 404 and alike exceptions might be more appropriate. For
OK-ish
> codes exceptions won't work. Maybe something along the lines:
> 
> @json_response
> def blah(...):
>   if stuff:
>     return {'msg': 'this would be code 200'}
>   else:
>     return JsonWithStatus({'msg': 'this is whatever'}, code=300)
> 
> (I'm a fan of strict typing whenever possible)
> 
> But it looks ugly...

Another idea is to return a tuple but it quickly can become messy due to
inconsistency between handlers.

lgtm

[iannucci, 2013-10-29 03:50:37]

On 2013/10/29 01:28:14, M-A wrote:
> On 2013/10/28 17:56:35, vadimsh wrote:
> > On 2013/10/28 15:37:23, M-A wrote:
> > > Vadim, what do you think?
> > 
> > I think it looks a little bit like a hack. But I don't have any solid
> > alternatives. For 404 and alike exceptions might be more appropriate. For
> OK-ish
> > codes exceptions won't work. Maybe something along the lines:
> > 
> > @json_response
> > def blah(...):
> >   if stuff:
> >     return {'msg': 'this would be code 200'}
> >   else:
> >     return JsonWithStatus({'msg': 'this is whatever'}, code=300)
> > 
> > (I'm a fan of strict typing whenever possible)
> > 
> > But it looks ugly...
> 
> Another idea is to return a tuple but it quickly can become messy due to
> inconsistency between handlers.
> 
> lgtm

Yeah I think the 'non-decorator' way to do this would be to just always wrap in
something like an HttpJsonResponse, but... meh. Verbose.

Another way would be to do:

StatusCode = object()

...

return {StatusCode: 204, ...}

Then it's not a string, and it's more out of band, since the raw object isn't
serializable to JSON anyway.

WDYT?

[iannucci, 2013-10-29 17:26:13]

ping :)

WDYT about the following idea:

StatusCode = object()

...

return {StatusCode: 204, ...}

Then it's not a string, and it's more out of band, since the raw object isn't
serializable to JSON anyway.

[M-A, 2013-10-29 18:06:53]

On 2013/10/29 17:26:13, iannucci wrote:
> ping :)
> 
> WDYT about the following idea:
> 
> StatusCode = object()
> 
> ...
> 
> return {StatusCode: 204, ...}
> 
> Then it's not a string, and it's more out of band, since the raw object isn't
> serializable to JSON anyway.

I like it. Then you can do:
if StatusCode in data:
 ...

I'd name it STATUS_CODE though.

[iannucci, 2013-10-29 18:41:36]

PTAL; Now with STATUS_CODE.

[M-A, 2013-10-29 18:48:43]

lgtm with optional nit.

https://codereview.appspot.com/17890043/diff/20001/codereview/views.py
File codereview/views.py (right):

https://codereview.appspot.com/17890043/diff/20001/codereview/views.py#newcod...
codereview/views.py:108: STATUS_CODE = object()
Thinking about it more, maybe JSON_STATUS_CODE to be extra clear?

[iannucci, 2013-10-29 18:50:27]

On 2013/10/29 18:48:43, M-A wrote:
> lgtm with optional nit.
> 
> https://codereview.appspot.com/17890043/diff/20001/codereview/views.py
> File codereview/views.py (right):
> 
>
https://codereview.appspot.com/17890043/diff/20001/codereview/views.py#newcod...
> codereview/views.py:108: STATUS_CODE = object()
> Thinking about it more, maybe JSON_STATUS_CODE to be extra clear?

verbooooossssseeee :(

Also we could theoretically use it for other stuff :p

[iannucci, 2013-10-30 03:14:40]

Committed as
https://code.google.com/p/rietveld/source/detail?r=06b194fe356e079d39fcd5c4dc...
(default)
Merged as
https://code.google.com/p/rietveld/source/detail?r=b55739cc37d67bbdc48caf74c0...
(chromium)