pando.py Use a single response object

Reticketed from https://github.com/AspenWeb/pando.py/issues/222#issuecomment-246205836:

Regarding the Response class, I think we should kill the raise Response(...) pattern. Instead of creating new response objects everywhere, we should only use the one in the state. That would allow us to give it access to the website and request objects, and thus provide a richer API.

Sep 12 '16 10:09 Changaco

Does anyone have objections? I'd like to do this before Pando 1.0.

Sep 14 '16 15:09 Changaco

Whether Response is raisable and whether it's global are separable concerns. Are you looking to change one, the other, or both?

Sep 14 '16 15:09 chadwhitacre

I'm not looking to prevent the raising of response objects, I'm only proposing that we use a single response object per request instead of creating new ones when raising.

Sep 14 '16 15:09 Changaco

Also, ideally the developer should be able to replace the Response class with a subclass, and that's not possible if we're creating new instances of the base class all over the place.

Sep 14 '16 15:09 Changaco

To clarify further, this is what I'm trying of get rid of: https://github.com/liberapay/liberapay.com/blob/134/liberapay/main.py#L139-L183

Sep 14 '16 15:09 Changaco

I'd like to still be able to raise Responses, but I'm fine to pass the class around somehow instead of using it globally as we've been doing.

Sep 14 '16 15:09 chadwhitacre

that's not possible if we're creating new instances of the base class all over the place.

It's not the creation of new instances that's the problem, but the global import. If we pass the Response class around in state and only instantiate the one pulled from there (rather than from pando import Response) then we could create multiple instances and still be able to control which class is in use.

Sep 14 '16 15:09 chadwhitacre

Yes, however subclassing Response isn't the primary reason why I want a single instance.

The first issue is that the Response class currently doesn't have access to the website and request objects, though this could be solved by changing raise Response(…) to raise request.Response(…), since there is only one request object and we could give it a reference to website.

The bigger problem is that when you're creating new instances you're throwing away every modification that previous functions may have made to the response object already in the state, such as adding cookies. I've hit this problem in Liberapay, and it seems to me that the best way to fix it is to use a single response object.

Sep 14 '16 16:09 Changaco

Hmm ... I would expect the opposite: website.Response(request). There's a new request for each request, but the same website. Likewise, there's one Response class, it doesn't change from request to request (does it?).

What does it look like to raise the response from a simplate?

if False:
    response.code = 400
    response.body = b'you bad'
    raise response

or maybe ...

    response.raise_with(400, b'you bad')

?

response(400, 'you bad') looks weird to me. I expect a lowercased callable to be a function, and I expect a function to have a verb for a name.

Sep 14 '16 16:09 chadwhitacre

response(400, 'you bad') looks weird to me. I expect a lowercased callable to be a function, and I expect function to have a verb for a name.

Then I suggest response.send(…). That function would raise the response, but if you want to make it clear that you're raising you can do raise response.send(…).

Sep 14 '16 16:09 Changaco

However, when you're doing raise Response(400, 'you bad') you don't actually want to send that response, you know it'll be picked up by delegate_error_to_simplate and another more complete response will be rendered using error.spt or 400.spt. So maybe response.error(…) would be better.

Sep 14 '16 16:09 Changaco

I do want to make it clear that I'm raising. What about framing it in terms of the request?

    raise request.terminate(400, 'you bad')

The point is that this is as far as the request is going to get. From here on out we're on our way back out to the browser.

v           ^     ^
 \         /     /
  \       /     /
   \     /     /
    \   /     /
     \ /     /
      Y     /
       \   /
        \ /
         V

Sep 14 '16 17:09 chadwhitacre

Or .end.

Sep 14 '16 17:09 chadwhitacre

    raise EndOfRequest(400, 'you bad')

Sep 14 '16 17:09 chadwhitacre

    raise request.End(400, 'you bad')

Sep 14 '16 17:09 chadwhitacre

I like that last one.

Sep 14 '16 17:09 chadwhitacre

What about framing it in terms of the request?

I think it's weird. It's like the defunct Request.redirect() function, while it's true that "redirect the request" makes sense, what the function actually does is manipulate a response object, so IMO it belongs in the Response class.

Sep 14 '16 17:09 Changaco

Okay. Well, we've got a few options out on the table. I'm happy to let you decide. :-)

Sep 14 '16 18:09 chadwhitacre

It just occurred to me that having a single response object is incompatible with #304.

Sep 16 '16 08:09 Changaco