-
Notifications
You must be signed in to change notification settings - Fork 244
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
Allow headers to be provided to fetch_url #499
Conversation
Hi @markevans, would you mind looking at this PR? Let me know if you think it needs further improvement. |
Hi @markevans, I'm upgrading the app where I use a fork of dragonfly with this PR applied. I notice you're still actively maintaining the gem so I wondered if you'd consider accepting this PR for inclusion? I'm happy to make any modifications you think necessary. |
Hi @markevans, is this something that we could still look to add? |
Hi - sorry for the unbelievably slow response on this! 😬 Looks good in principle, however, I don't think we can include this because of the following reason (sorry I didn't think about this before)... The job steps, including the arguments, get encoded into the url, so for example job = Dragonfly.app.fetch_url("https://example.com/image")
job.url # "/W1siZnUiLCJodHRwczovL2V4YW1wbGUuY29tL2ltYWdlIl1d/image?sha=033ecdd26d3968ad" job = Dragonfly.app.fetch_url("https://example.com/image", headers: {"authorization"=> "Bearer abc123"})
job.url # "/W1siZnUiLCJodHRwczovL2V4YW1wbGUuY29tL2ltYWdlIix7ImhlYWRlcnMiOnsiQXV0aG9yaXphdGlvbiI6IkJlYXJlciBhYmMxMjMifX1dXQ/image?sha=4cf8250ff0fa8974" (note the longer url). As you can see the url encodes the auth details - this can even be decoded: Dragonfly::Serializer.json_b64_decode("W1siZnUiLCJodHRwczovL2V4YW1wbGUuY29tL2ltYWdlIix7ImhlYWRlcnMiOnsiQXV0aG9yaXphdGlvbiI6IkJlYXJlciBhYmMxMjMifX1dXQ") # ==> [["fu", "https://example.com/image", {"headers"=>{"Authorization"=>"Bearer abc123"}}]] For your app a better idea would be to retrieve the image yourself using whichever http library, then if assigning to an activerecord model do it as per https://markevans.github.io/dragonfly/models#using-the-accessors. To be honest if I were to start again I may not have even included Apologies I didn't really have time to think this through previously |
Hi @markevans, absolutely not a problem! :D I'm a gem maintainer myself and know how real paying works takes priority :) I didn't realise it worked this way. I assume the fetch only happens once at the time the record is assigned and then Dragonfly stores its own copy of the image? This is my use case (ActiveRecord):
The
Is the concern just that someone with access to the job queue could acquire the token? As you say though, it's easy enough to fetch the file using Faraday or something like that instead and sidestep all of this. |
In your case, yes the fetch is only happening when assigned and Dragonfly is storing its own copy (the line But Dragonfly provides extra functionality such that url = Dragonfly.app.fetch_url("https://some.url/image").url # --> "/adsfkladfk..." If you were to visit this url Dragonfly would fetch that url and serve it (you can chain on processing steps etc.). However it's not really in Dragonfly's remit to provide extra HTTP functionality beyond v. basic as this can easily be done with Faraday or something as you mentioned. I think the best bet would probably be to replace the line self.file = Dragonfly.app.fetch_url(... with self.file = SomeLibraryLikeFaraday.get("... Hope that makes sense |
Thanks @markevans, that definitely makes sense. I suspected it would be something like that. Thanks for the detailed explainer :) Above and beyond! All the best :D |
Closes #498
Allows optional headers to be passed to the request. The interface for headers is a bit weird so we need to loop the passed in headers and then add each one using a hash-like
[]
interface. It seems they can't be all added at once.