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

API to post file #73

Closed
sferik opened this issue Jan 30, 2014 · 24 comments · Fixed by #167
Closed

API to post file #73

sferik opened this issue Jan 30, 2014 · 24 comments · Fixed by #167
Assignees
Milestone

Comments

@sferik
Copy link
Contributor

sferik commented Jan 30, 2014

I’m trying to perform a multipart POST with the HTTP gem but I’m having trouble getting the headers and body right. We should include an example of how to do this in the README.

@tarcieri
Copy link
Member

Or possibly an API like #post_file

@sferik
Copy link
Contributor Author

sferik commented Jan 30, 2014

@tarcieri Yes, that would be nice.

FYI, I’ve been working on replacing Faraday with the HTTP gem on a branch in the Twitter gem. I’ve got pretty much everything working except for posting files (this branch also depends on JSON parsing, which I’ve hacked together in a local branch).

@tarcieri
Copy link
Member

Nice, someone actually pinged me on Faraday support for the HTTP gem yesterday

@sferik
Copy link
Contributor Author

sferik commented Jan 30, 2014

That would be nice too.

I haven’t done proper benchmarking but the Twitter test suite (about 900 examples that make about 368 HTTP requests) runs about 4X faster in the http branch than in master (with Faraday). Obviously, the bottleneck of any HTTP API wrapper is network latency but this seems like a pretty significant reduction in request overhead.

I’d be curios to see how the performance would compare using an HTTP gem adapter for Faraday. I suspect it would be slower than using just the HTTP gem (sans Faraday). I think Faraday’s slowness is a result of having a middleware stack that each request must pass through.

I do think writing a Faraday adapter would make it easier for people to try the HTTP gem in their existing clients.

@Asmod4n
Copy link
Contributor

Asmod4n commented Feb 3, 2014

https://github.com/nicksieger/multipart-post actually looks like it can be used anywhere.

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2014

I don't think this is required for 0.6

@sferik
Copy link
Contributor Author

sferik commented Mar 4, 2014

I agree, this is more of a nice-to-have for v0.6 but I think it’s a requirement for v1.0.

@sferik sferik modified the milestones: v1.0, v0.6 Mar 4, 2014
@sferik
Copy link
Contributor Author

sferik commented Mar 4, 2014

I’ve moved this issue out of the v0.6 milestone and created a new v1.0 milestone.

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2014

Seems good

@ixti
Copy link
Member

ixti commented May 25, 2014

An issue with #post_file is that it will make API call look pretty strange when you need to upload more than one file or when you need to post some form values as well. I propose to stay with the same API we have now, we just need to recognize part of :form:

HTTP.post(url, :form => {
  :title => "Foobar",
  :file => HTTP::UploadIO.new(path_to_file, optional_mime_type)
})

Something very similar to Faraday actually ;))

@ixti ixti added the Pick Me! label May 25, 2014
@sferik
Copy link
Contributor Author

sferik commented May 26, 2014

@ixti That would be acceptable to me. What is needed to support this?

@ixti
Copy link
Member

ixti commented May 26, 2014

@sferik I guess some spare time ;)) Other than that I don't see any problems here :D

@tarcieri
Copy link
Member

@ixti 👍

@ixti ixti removed the Discussion label May 26, 2014
@sferik
Copy link
Contributor Author

sferik commented Nov 10, 2014

@Asmod4n multipart-post uses Net::HTTP 😝

@ixti I’ve merged the http branch into master. This issue is the only thing stopping me from releasing the next major version of the twitter gem. Any idea when you might get a chance to work on this?

@ixti ixti self-assigned this Nov 10, 2014
@ixti
Copy link
Member

ixti commented Nov 12, 2014

@sferik will try to jimp on this during this weekend.

@sferik
Copy link
Contributor Author

sferik commented Nov 12, 2014

@ixti \o/

@JuanitoFatas
Copy link
Contributor

@ixti 👍

@Asmod4n
Copy link
Contributor

Asmod4n commented Nov 12, 2014

@sferik what i was trying to say was multipart-post looks like it doesn't depent on anything and can be injected into any IO library, namely https://github.com/nicksieger/multipart-post/blob/master/lib/parts.rb seams to be independent of Net::HTTP and can be hooked into this gem :)

@sferik
Copy link
Contributor Author

sferik commented Nov 12, 2014

@Asmod4n Ah, very nice. :)

@ixti
Copy link
Member

ixti commented Nov 17, 2014

I have started working on this issue, but not yet finished :D Will finish during this week.
We will not use multipart-post. Although it's really might be possible to couple it with HTTP gem, I dislike it's code and would like to come with our own solution.

@sferik
Copy link
Contributor Author

sferik commented Nov 17, 2014

@ixti Awesome! If this code is independent of the http gem, do you think it makes sense to package it as a separate gem (suggested name: http-multipart)? This would force us to have a clean separation and allow people to use it with other Ruby HTTP libraries.

@ixti
Copy link
Member

ixti commented Nov 17, 2014

@sferik Makes sense. Although I would like to first build it as part of HTTP gem - and then simply extract those classes out :D

Re suggested name. Actually a more appropriate name will be http-form or probably http-form_data. A sneak peek to the API:

# Usage inside with HTTP Gem:

HTTP.post(some_url, :form => {
  :username => "ixti",
  :avatar => HTTP::FormData::File.new("/path/to/file.jpg"),
  :cv => HTTP::FormData::File.new(io, :filename => "cv.pdf")
})

# Standalone usage:

form = HTTP::FormData.new({
  :username => "ixti",
  :avatar => HTTP::FormData::File.new("/path/to/file.jpg"),
  :cv => HTTP::FormData::File.new(io, :filename => "cv.pdf")
})

form.to_s # => returns HTTP request multipart body

@zackxu1
Copy link

zackxu1 commented Nov 10, 2016

@ixti The above is really helpful. It would be even more helpful if it's in the README of the gem. Thank you.

@tarcieri
Copy link
Member

@zackxu1 it's documented here: https://github.com/httprb/http/wiki/Passing-Parameters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants