Add DownloadStrategy with custom :headers support #1944
Conversation
|
Wow this is great work! I've been meaning to do this for quite some time. |
|
No sabia que usabas el proyecto. Thanks for the contribution, let me look over this and do some testing. |
| end | ||
| downloader = CurlDownloadStrategy.new(cask.title, resource) | ||
| end | ||
| downloader = Cask::DownloadStrategy.new(cask.title, resource, cask.headers) |
There was a problem hiding this comment.
Now that we own the DownloadStrategy interface, can can make it just take (cask, resource) and then have the DownloadStrategy retrieve the data it needs directly from the Cask?
There was a problem hiding this comment.
You're right! I can fix it :)
|
👍 Just found about it Today while looking to install vagrant (thanks @phinze). Queria contribuir algo :) |
| @@ -27,6 +36,14 @@ def url(url=nil) | |||
| @url ||= Cask::UnderscoreSupportingURI.parse(url) | |||
| end | |||
|
|
|||
| def headers(headers=nil) | |||
| @headers ||= OpenStruct.new(headers) | |||
There was a problem hiding this comment.
Is there anything gained by the OpenStruct here other than dot notation? It seems like the marshall_dump stuff is extra work that would fall away if we just switched to a straight hash here.
If interaction with a hash gets hairy, I'd recommend spinning up a new Cask::Headers object to encapsulate the data.
There was a problem hiding this comment.
Other than the dot notation, there's actually nothing else. I like the idea of having Cask::Headers I had thought about this but was reluctant to keep adding classes but for this I think it makes sense. I have some free time Tonight so I'll get to this and the previous change suggested ASAP.
There was a problem hiding this comment.
Cool - no rush of course - this is FOSS. 🐵
Feel free to hop in #homebrew-cask on freenode if you have any other questions.
|
btw, with these changes |
Add DownloadStrategy with custom :headers support
|
Landed with some refactoring. Working on a few docs now. |
This adds support for overriding the default UserAgent that curl adds by default and also accepts a :cookies hash.
@nanoxd - #1175 called for a DownloadStrategy with UserAgent support to avoid problems related to 403's / Spam in Detection like #984, but after seeing so many brackets pull-requests that fail and a few issues related to being limited by the original
CurlDownloadStrategy; I think it would be good to incorporate a simple headers option that can be extended as needed.A few examples:
For a simple UserAgent, you can use the standalone :fake_user_agent that impersonates Chrome.
fake_user_agentThe :cookies option is based on #958
which has been sitting for 2 months with no answer from the author so I went ahead and did some small changes and used it.
References #1175