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

Proposal: expose a public S3 module #446

Open
benjreinhart opened this issue Jan 9, 2025 · 2 comments
Open

Proposal: expose a public S3 module #446

benjreinhart opened this issue Jan 9, 2025 · 2 comments

Comments

@benjreinhart
Copy link
Contributor

benjreinhart commented Jan 9, 2025

Thank you for your work on Req!. I wanted to suggest an area for improvement:

I want to use the S3-related logic in Req.Utils in my own application for presigning, but that module is not publicly documented which leads me to believe its use is discouraged. However, it is used externally by at least ReqS3. The bulk of S3 logic lives in the Req.Utils module, with ReqS3 a relatively small wrapper around it. All this to say, the boundaries between the two packages are unclear and I'm not sure that a ReqS3 package is necessary if the most important part to encapsulate is provided by Req (i.e., signing utilities).

My proposal is to create an officially supported public interface for S3-related functionality. The exact scope of that module is up for debate, but at a minimum it would need to provide a complete implementation of AWS Signature Version 4. Beyond that, we could consider bringing some of the ReqS3 utilities in, or possibly moving the put_aws_sigv4/1 to this module. Most important is the signing code, which is the foundation everything else needs.

The goals are:

  1. Expose AWS Signature Version 4 (#443 and #445 bring us closer to a complete implementation)
  2. Make a clear interface external code can rely on.
  3. Clarify boundaries between Req and ReqS3
  4. Optionally provide other S3 utiltiies, e.g., presign_url or handle_s3_url step.

Since S3 is ubiquitous in the Cloud, I think a high percentage of projects in the ecosystem would benefit from their HTTP client providing robust utilties for interacting with it.

If agreed this is a good direction, I can open a PR.

@wojtekmach
Copy link
Owner

Hey, I agree it is in a weird spot right now. I was thinking the other day about the opposite, though, moving AWS stuff completely out of Req to ReqS3. So the idea of having AWS signature in Req was, following curl --sign-awsv4, that this is ubiquitous, it'd work for S3 as well as other AWS requests one might do as well as all the S3-compatible services out there. I'm not sure if using this signature for non-S3 really happened though. And so my rationale with potentially removing AWS from Req is, if you're using S3 you may want to use req_s3 for other stuff it has (presign_form, presign_url, s3:// pseudo protocol support), and if you use other AWS service you could do ReqS3.put_aws_sigv4 or something.

That being said, yes, my goal with Req is supporting what most people tend to use most of the time and S3 fits that. I don't think requiring people to use a plugin adds a lot friction so I'm kind of OK with the status quo (messy internal boundaries not withstanding) but I'd be lying if I told you I haven't thought about bringing everything in into Req proper. I'll give it some more thought, thank you for bringing this up.

If you need just signing, another option for you is https://hexdocs.pm/aws_signature/aws_signature.html (which Req optionally depended on until I re-implemented it (with many bugs)). I think this is a solid package, though if you end up using it consider proposing deprecating sign_v4/10 and friends in favor of sign_v4(map), I'm sure they would not mind, and it would significantly improve ergonomics of using it. :)

@benjreinhart
Copy link
Contributor Author

I'm not opposed to moving everything into req_s3, in most ways I think that is better. I proposed this assuming you wanted to keep signing and put_aws_sigv4/1 in Req proper, and if that were to be the case I would like to mature+expose it. If that's not true, then moving the S3 logic out of Req into ReqS3 makes sense and I'd be happy to help with that as well. It's the split that's confusing.

If you need just signing, another option for you is https://hexdocs.pm/aws_signature/aws_signature.html

For sure, but I already use Req and it has almost everything I need (or at least, should with #445). At which point, I'd like to use an existing dep of mine :)

Happy to help with this if you decide on next steps

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

No branches or pull requests

2 participants