Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP.verb(url) should return an HTTP::Response object by default, not the body #37

Closed
tarcieri opened this issue Sep 14, 2013 · 9 comments

Comments

@tarcieri
Copy link
Member

The current API is probably overly simplified:

body = HTTP.get(url)

To get the response object, we have to do this:

response = HTTP.get(url).response

Really it should probably be:

response = HTTP.get(url)
body = response.body

It'd be nice if body were an HTTP::ResponseBody object as well, then we could support streaming like this:

body = HTTP.get(url).body
chunk1 = body.readpartial
chunk2 = body.readpartial

We could also make it Enumerable so you can use it like this if you prefer:

body = HTTP.get(url).body
body.each do |chunk|
  ...
end
@jarinudom
Copy link

I actually prefer it the way it is, since like 90% of the time you just want the body anyway.

I'm not married to either way though, I'll use it regardless.

@tarcieri
Copy link
Member Author

The current API has a lot of limitations, especially around streaming. I do like the convenience which is what I was originally going for. Perhaps there can be convenience methods to get to the body in an easy way

@jarinudom
Copy link

I really like the AREL-style method chaining and I think it is one of the major draws of this gem (to me, anyway), so maybe in uncertain cases it would be good to look at whatever AREL's philosophy is (which I think would probably point to the HTTP.get(url).body method you pointed to above).

@sferik
Copy link
Contributor

sferik commented Sep 14, 2013

We could alias HTTP::Response#to_s to HTTP::Response#body such this would return the response body as a string:

"#{HTTP.get(url)}"

@tarcieri
Copy link
Member Author

We'd need to delegate HTTP::Response#to_s to HTTP::ResponseBody#to_s actually.

I have a broken WIP implementing some of these changes I can link which might help clarify this stuff ;)

@sferik
Copy link
Contributor

sferik commented Sep 14, 2013

Yes, that’s what I meant.

@tarcieri
Copy link
Member Author

Code speaks louder than words. Here's a WIP. I need to finagle it a little more, pulling in some of the other stuff from Reel: #39

@ghost
Copy link

ghost commented Sep 15, 2013

haven't used this gem yet but i think i'd prefer HTTP.verb() to return a response.
body = HTTP.get(…).body looks good to me.

@sferik
Copy link
Contributor

sferik commented Jan 23, 2014

This can be closed, as #39 has been merged.

@sferik sferik closed this as completed Jan 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants