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

Remove the dependency on Faraday #60

Closed
hajee opened this issue May 22, 2015 · 7 comments · Fixed by #61
Closed

Remove the dependency on Faraday #60

hajee opened this issue May 22, 2015 · 7 comments · Fixed by #61

Comments

@hajee
Copy link
Contributor

hajee commented May 22, 2015

For our use case it is difficult to get an extra gem installed. At the moment, this means we cannot use the module. I would like to use at least the types (and providers) in this module, but they are 'hard wired' using faraday.

I've looked at the code and it seems you are using faraday mostly as an easy way to call NET:HTTP. I could try and make a PR removing Faraday from the provider. Is this beneficial? Am I missing something in the reasoning for Faraday?

@nanliu
Copy link
Contributor

nanliu commented May 22, 2015

I would like to switch to net::http, we can remove faraday once we implement a proper redirect:
https://github.com/lostisland/faraday_middleware/blob/master/lib/faraday_middleware/response/follow_redirects.rb

@hajee
Copy link
Contributor Author

hajee commented May 22, 2015

In older issues I saw you also had ideas to remove faraday. Did you start with this?

@nanliu
Copy link
Contributor

nanliu commented May 22, 2015

I started working on #55, but got hanged up because of the redirect issue.

@jairojunior
Copy link

+1

Perhaps a simpler solution is enough:

http://stackoverflow.com/questions/6934185/ruby-net-http-following-redirects?answertab=active#tab-top

We only need to support HTTP GET, right?

@hajee
Copy link
Contributor Author

hajee commented May 26, 2015

I've been looking into this. I'm guessing issue #55 is caused by the standard Net::HTTP class and not by Faraday. It looks like Net::HTTP is reading the whole response object into memory, causing memory problems when working with big files. So when we replace Faraday by our own solution, we also have to be careful not to keep the same problem.

Wouldn't it be easier to use wget. This would still introduce a dependency, but this is a rpm dependency that, most of the times is much easier. The advantage would be a battle tested en tried http/ftp and https implementation.

It supports a lot of stuff, for example:

  • retries
  • limit rate. This could be useful when downloading files on fragile networks
  • proxies
  • redirects

@nanliu
Copy link
Contributor

nanliu commented May 26, 2015

This module was originally written to handle both windows/linux. A native ruby solution worked better than being dependent on a package that I could not install in windows. Both wget/curl can be implemented as providers to supplement the default provider (or even default providers on Linux). The util portion can be moved to download method so the default provider can be the parent provider with the download method replaced with your preferred solution on platforms where it's suitable.

@hajee
Copy link
Contributor Author

hajee commented May 27, 2015

Sorry. Forgot about Windows for a moment ;-). I'll have my stab at making a provider for unix wget.

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

Successfully merging a pull request may close this issue.

3 participants