ex-aws / ex_aws_s3

170 stars 147 forks source link

ex-aws configuration is not used for presigned_url/4 #4

Open rsullivan00 opened 6 years ago

rsullivan00 commented 6 years ago

Environment

Current behavior

Currently, ExAws.S3.presigned_url requires a config map to be manually passed in to access things like our secret access key.

S3.presigned_url(
  ExAws.Config.new(:s3),
  :get,
  ...
),

It seems like this should happen implicitly using our ExAws S3 configuration, since other functions use our configuration automatically. For example,

ExAws.S3.list_buckets

Discovers our configuration internally.

Expected behavior

presigned_url behaves like other functions by discovering configuration

benwilson512 commented 6 years ago

Technically they do not use configuration automatically. Configuration is passed in to the ExAws.request/1,2 functions. The functions within the S3 module really don't use config at all with the one exception of the presigned_url fun.

I can look and see though if that config can be used by default.

janajri commented 6 years ago

Just encountered this issue, but had no issues passing in the config map returned from ExAws.Config.new(). Could be useful to just add another f

@spec presigned_url(http_method :: atom, bucket :: binary, object :: binary, opts :: presigned_url_opts) :: {:ok, binary} | {:error, binary}

Happy to make a PR if you think it makes sense.

aisrael commented 4 years ago

Was trying to figure this out just now as well. At first I tried ExAws.Config.Defaults.default(:s3) but that gave a validation error.

@janajri's comment pointed the way out to me:

:s3 |>
ExAws.Config.new() |>
ExAws.S3.presigned_url(:put, "bucket", "object")
aisrael commented 4 years ago

I think a better approach would be to simply make ExAws.S3.presigned_url create the request, then the user has to pipe it to ExAws.request() to be consistent with the rest of the library?

benwilson512 commented 4 years ago

The issue is that it isn’t a request, nor is a pre-signed URL an operation. I agree that the API is inconsistent, which is definitely sub optimal, but piping it though request isn’t really consistent either.

On Thu, Nov 21, 2019 at 8:00 PM Alistair A. Israel notifications@github.com wrote:

I think a better approach would be to simply make ExAws.S3.presigned_url create the request, then the user has to pipe it to ExAws.request() to be consistent with the rest of the library?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ex-aws/ex_aws_s3/issues/4?email_source=notifications&email_token=AAB4QCMTVWUZUETVTP2BCKDQU4VLZA5CNFSM4EDSP22KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE4FQHY#issuecomment-557340703, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4QCJCV37IF6XZRF2DK5TQU4VLZANCNFSM4EDSP22A .