-
Notifications
You must be signed in to change notification settings - Fork 11
Allow passing custom query params #8
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
Comments
Hey Venkat! Which functions are you using? These should both work today: Req.get!(req, url: "s3://bucket1?versions")
ReqS3.presign_url(url: "s3://bucket1?versions") The only one without a workaround is In any case, yes, we should definitely support it, PRs would be very appreciated. I might end up regretting it in the long run, but I'd prefer to keep majority of AWS signature logic in Req proper since we use some of it there. We should add a :query or similar option. |
Edit: nevermind, |
Hi! I was using The use case is that our app has an option to either download a file or view it in a PDF viewer directly in the browser. So I need to generate two pre-signed urls: The one that forces a download would ideally look like so: ReqS3.presign_url([
bucket: bucket,
key: key,
query: [{"response-content-disposition", "attachment; filename=#{filename}"}]
] ++ s3_options) I'm happy to give a PR a try later this month. I need to study the ex_aws implementation a bit because there are some details on how to sort the query params. It seems like the bulk of the changes would be in |
Sounds good! |
I am trying to move to a lighter abstraction and req_s3 seems great!
One thing in the way of that is that I am not able to pass in custom query_params.
You can see it's implemented here in ex_aws:
https://github.com/ex-aws/ex_aws/blob/8020fa1c0a7221c3931a91ec8089e52c868667c0/lib/ex_aws/auth.ex#L76
It gets merged into the canonical params. I noticed Req.Utils contains the logic to sign S3 requests, but perhaps that should belong in req_s3 before any improvements are made?
The text was updated successfully, but these errors were encountered: