PreparedRequest.dump()

[kennethreitz]

>>> r = requests.get('http://httpbin.org/ip')

>>> print r.request.dump()
REQUESTS/2.9.1 GET http://httpbin.org/ip
Connection: keep-alive
Accept-Encoding: gzip, deflate
Accept: */*
User-Agent: python-requests/2.9.1

[kennethreitz]

I think render is a good name if it was reporting HTTP/1.1 in the strings. Another name may be more appropriate now.

[kennethreitz]

I find it entertaining how this showcases just how much we completely disregard header order.

[kennethreitz]

Thinking of removing Response's render.

[Lukasa]

Maybe dump()?

[kennethreitz]

@Lukasa hmmm, that works.

[EmilStenstrom]

This looks exactly like what I was hoping for.

A couple of comments (feel free to disregard and ship anyway, I'm already happy):

1. I'm not sure about the name render(). The method feels like a variant of __str__ but with all the gory details. I expect it to be used mainly for debugging, so maybe just debug() instead?
2. I really would like both of them (Response.render() and Request.render()). For debugging purposes I often need both the sent request and the response. I then take both of them and send them in an e-mail to the one with the server, so they can debug why my requests are not working right. Without a way to render responses I still would have to have my own utility for that.
3. Not that fond of the "REQUESTS/2.9.1" string. Maybe we could do PreparedRequest<[string_here]> as a way to mimic __repr__?

[kennethreitz]

@EmilStenstrom render has been renamed to dump.

[EmilStenstrom]

dump() works for me!

[sigmavirus24]

What if body is a file-like object? This won't render the way you expect.

[Lukasa]

The same is true for generators and other weird iterators.

[sigmavirus24]

That is to say, it doesn't make sense to handle this differently from how the toolbelt already handles this. We puzzled out how to do this properly over there months ago and the code should be liftable to avoid having to reinvent the wheel and catch the same problems.

[kennethreitz]

Currently, they're represented by their repr, which seems like a perfectly fine solution to me. What behavior does the toolbelt exhibit in that scenario?

[sigmavirus24]

Not that fond of the "REQUESTS/2.9.1" string. Maybe we could do PreparedRequest<[string_here]> as a way to mimic repr?

I agree with @EmilStenstrom. I don't like that much either. It's confusing and may make some people think we're doing some weird protocol instead of HTTP.

[EmilStenstrom]

Concrete suggestion of syntax for dump():

>>> import requests
>>> r = requests.get('http://httpbin.org/ip')
>>> r.request
<PreparedRequest [GET]>
>>> r.request.dump()
<PreparedRequest [
    GET http://httpbin.org/ip
    Connection: keep-alive
    Accept-Encoding: gzip, deflate
    Accept: */*
    User-Agent: python-requests/2.9.1
]>

[EmilStenstrom]

Any rationale for removing Response.dump()? I find it really useful for debugging a remote service in the same way that Request.dump() is useful.

[kennethreitz]

@EmilStenstrom hmm, that's not a bad way to present it! I like it.

For the record, I'm still not 100% on including this. I'm still tossing the idea back and forth in my mind. This syntax definitely helps, though.

This syntax also helps support the inclusion of Response.dump. It just didn't make sense with the other syntax.

[kennethreitz]

@EmilStenstrom updated the syntax.

<PreparedRequest [
    POST http://httpbin.org/post
    User-Agent: python-requests/2.9.1
    Connection: keep-alive
    Accept-Encoding: gzip, deflate
    Accept: */*
    Content-Length: 3447
    Content-Type: multipart/form-data; boundary=bf96155646964bf082b7a30531aa7c7e
]>

[kennethreitz]

Using colors to display warning about non string-like content bodies now. I like where this is going.

Things to do:

• disable color output when not a tty
• potentially color the rest of the output, not sure how i feel about that.

[kennethreitz]

Ooooh, it's so pretty (and cross-platform).

I dig this.

[EmilStenstrom]

I'm very happy with how this turned out, and by also including Response.dump() all three of my concerns have been delt with. It's ready to ship in my mind.

[kennethreitz]

@EmilStenstrom as soon as I'm done perfecting PreparedRequest, I'll move on to Response :)

[StewPoll]

TetraEtc for that, there is pretty(colors=False). Also, colors are automatically disabled if the code is not running in an interactive console.

That's why I don't care that much either way. It's not something I'd use much. I care about the entire feature, not the colouring specifically

[StewPoll]

This is already the case. There is code you can easily utilize to do this, albeit in a more accurate and potentially more verbose way.

It's entirely possible I'm simply just not aware of how to do this, but this appears to be an incredibly easy way of doing this when needed.

[alexwlchan]

Nitpick: "seperated" ~> "separated"

[kennethreitz]

Thanks!

[sigmavirus24]

It's entirely possible I'm simply just not aware of how to do this, but this appears to be an incredibly easy way of doing this when needed.

https://toolbelt.readthedocs.org/en/latest/dumputils.html

[sigmavirus24]

Also worth noting that we're re-implementing part of https://github.com/jkbrzt/httpie

[kennethreitz]

Random thoughts:

• it's so pretty!
• i have absolutely no problem including this, in any way shape or form
• however, requests is an intensely massive project, so "playing it safe" is probably best
• this annoys me, and i'm not sure how I feel about it
• therefore, I may do it anyway
• therefore, I may not
• my cat loves boxes

No further 👍s or 👎s needed at this point, but random anecdotes are more than welcome!

[ghost]

My cat loves leather chairs. Doesn't like boxes at all.

[EmilStenstrom]

When thinking about this, know that there are two different features included here:

1. Dumping a request or response as a string
2. Making said string pretty.
   I think accepting 1 should be considered playing it safe.

[kennethreitz]

@EmilStenstrom absolutely, that'll happen no matter what. :)

(unless I change my mind, for some reason)

[kennethreitz]

I don't actually think including the pretty formatting matters in any way, shape, or form, to be completely honest. Meaning, I see absolutely no reason not to. I think it's a nice feature, and I would be very pleasantly surprised to find that level of polish in my favorite HTTP library.

That being said, I'm taking extra time to be considerate of @Lukasa and @sigmavirus24's objections (regardless of final result).

[kennethreitz]

My cat also loves hiding inside my recliner.

[sigmavirus24]

I would be very pleasantly surprised to find that level of polish in my favorite HTTP library.

And I'd be very confused why an HTTP library is doing anything with colorized output. ¯\_(ツ)_/¯ I think we have different definitions of the word "polish" w/r/t what a library should do.

I'm taking extra time to be considerate of @Lukasa and @sigmavirus24's objections (regardless of final result).

Sounds like you're going to merge this regardless of our objections? If so, I'm not sure why you would take extra time.

[alexwlchan]

I like the idea of pretty-printing request bodies, but I’m uneasy about colorised output, at least in the core library.

A few disconnected thoughts:

• This seems like something that might be useful when I’m in an interactive session (colorisation and all). But most of the time I’m not in an interactive session; requests is part of a larger application and is just one piece of the whole. It churns away in the background; nobody is inspecting the raw HTTP requests.

  Pretty-printing is less useful there. I’m not going to log the details of every HTTP header request.

• I tend to agree with others that this pretty-printing, especially colorisation, feels a bit odd in a library for handling HTTP requests.

  Which is a shame, because it seems like something I’d find useful on occasion – but it doesn’t feel like a core function.

• Might it be worth pushing into requests_toolbelt instead (at least the colorisation aspect)? Seem like that might be a better home for this.

• With my corporate hat on, vendoring in an extra module will probably cause hassle in environments where you need legal sign-off for new third-party software (especially since colorama has a different license to requests). I would rather avoid that hassle – especially if it’s for a feature that’s orthogonal to core HTTP, and which will never actually be used!

• My cat loves pawing at cardboard flaps, especially the ones you get on Amazon book packaging.

[kennethreitz]

@sigmavirus24 if I was sure, I would have merged it a week ago :)

[kennethreitz]

e.g. I am currently leaning towards inclusion, while swaying back and forth. Simply using this thread to think out loud.

[kennethreitz]

My cat enjoys long naps on flannel blankets on couches.

[kennethreitz]

My 6-year-old goldfish is currently living in a small man-made pond at my ex-girlfriend's house.

I regret that decision :P

[sigmavirus24]

I avoid adopting pets because open source projects need just as much love and are just as alive (or so I try to convince myself). ;)

[Lukasa]

So, while we're taking feedback, here's my 2¢:

The philosophy of this project since about v1.0, at least as I understand it, has been to aggressively resist scope creep. We have rejected innumerably more feature requests than we've accepted, saying "no" to almost all feature requests that failed to meet one of three criteria:

1. It's extremely difficult or impossible to implement the feature from outside the library.
2. It's a convenience feature that would be used by a substantial majority of our users.
3. There is some subtlety in "correctness" of the feature that many users would miss, and that we have an opportunity to get right for them.

As far as I can see this feature fits into none of those categories.

To the first point, it is not difficult to implement this feature from outside Requests: it is exactly as hard there as it is here. The objects being used here are public and are regularly exposed to users: there's nothing that users would ordinarily be unable to find or struggle to reach.

To the second point, this feature would not be used by a majority of our users. The colorizing feature in particular suffers here because it only works when printing directly to a terminal and having a user observe those responses. That is a vanishingly small use-case of requests compared to automated use, and while it's important that those users have a good experience, I don't see value in implementing something solely for them unless it meets criteria 1 or 3. The dumping feature, even ignoring colorizing, is also something that is unlikely to be used by a substantial majority of our users, particularly because it is thoroughly ill-suited to logfiles. This is for two reasons: first, the dump contains newlines which make parsing logfiles extremely unpleasant; and second, the dump is really quite verbose and contains a lot of extraneous information that will serve only to bloat log files.

To the third point, the only subtlety in correctness here is not dealt with by this code, it is encouraged by this code, and that is the fact that the representation dumped here is not the representation as received from or sent to the network. It is related to those representations, but in no way conforms to them. Users who do not grasp this subtlety are, in my opinion, likely to be confused by the representation used here. The example used in the original post is great for this:

REQUESTS/2.9.1 GET http://httpbin.org/ip
Connection: keep-alive
Accept-Encoding: gzip, deflate
Accept: */*
User-Agent: python-requests/2.9.1

In this example the Host header is missing entirely, and the request URI is not what is actually sent on the network (that would be /ip, unless a proxy is in use in which case it is what would be sent on the network, unless we're making a HTTPS requests and then there's a whole separate web request that we aren't showing at all here). I should note also that the REQUESTS/2.9.1 block is not in the same place as the HTTP/1.1 block would be in a request, though I don't know if that's intentional or not. The Response suffers even more from this because the response headers have been thoroughly transformed by the time they get to requests and frequently look almost nothing like they did when they were received from the network.

All of that goes to say that I don't believe we'd accept this feature if it came from outside the project. I think it represents a maintenance burden for the future, because once we implement this we'll need to support it, and I don't believe that it pays for that burden in utility. And I haven't even begun to mention that to implement this feature we are adding two implicit dependencies that will now require updates in order to fix bugs that affect this rendering and that will create substantial work downstream of us to unbundle.

Generally speaking we have been fairly aggressive about getting features out of this library that are not required. I think this represents a step in the other direction, and I don't believe that we gain enough by doing it to justify it.

Of course, @kennethreitz, this is still your project and so my opinion is only that. =) If you merge this, I'll help maintain it.

[kennethreitz]

@Lukasa failurel! The above post stated:

No further 👍s or 👎s needed at this point, but random anecdotes are more than welcome!

My cat won't leave me alone today.

[jwg4]

It seems to me that '2. It's a convenience feature that would be used by a substantial majority of our users.' is a no-brainer. Every Requests user who makes mistakes would like to see this output in a debugger.

[sigmavirus24]

Thanks for playing @jwg4 but the instructions in this thread clearly state that only random anecdotes are welcome at this point.

[kennethreitz]

You have a cool avatar @jwg4.

[piotr-dobrogost]

https://en.wikipedia.org/wiki/Sierpinski_triangle

[justanr]

I'd like to see this included, right now I'm logging requests with a rather naive:

# inside of session
def send(self, request, ...):
    self._logger(self._level, "Sending request: {}".format(pformat(request.__dict__))
    resp = super().send(request, ...)
    self._logger(self._level, "Got back: {}".format(pformat(resp.__dict__))
    return resp

As for the cat tax, my cat enjoys chasing my dog off her bed.

[kennethreitz]

Closing, for now. I still think this is useful, and the branch still exists. But, something just doesn't feel right.