ex_aws_s3 icon indicating copy to clipboard operation
ex_aws_s3 copied to clipboard

upload/4 result is not parsed

Open khaskelberg opened this issue 5 years ago • 1 comments

  • Do not use the issues tracker for help or support (try Elixir Forum, Slack, IRC, etc.)
  • Questions about how to contribute are fine.

Environment

  • Elixir & Erlang versions (elixir --version):
Erlang/OTP 22 [erts-10.5] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe]

Elixir 1.9.1 (compiled with Erlang/OTP 21)
  • ExAws version mix deps |grep ex_aws
* ex_aws 2.1.1 (Hex package) (mix)
  locked at 2.1.1 (ex_aws) 1e4de210
* ex_aws_s3 2.0.2 (Hex package) (mix)
  locked at 2.0.2 (ex_aws_s3) c0258bbd
  • HTTP client version. IE for hackney do mix deps | grep hackney
* hackney 1.15.1 (https://github.com/benoitc/hackney.git) (rebar3)

Current behavior

When we use upload/4 method response body is not parsed, because parse_complete_multipart_upload does nothing https://github.com/ex-aws/ex_aws_s3/blob/10c0ff5bd6af3644759ecdd7f3f15a9de35a8a4f/lib/ex_aws/s3/parsers.ex#L83

Expected behavior

Response body is parsed

Proposed Solution

Right now parse_upload method parses CompleteMultipartUploadResult and is not used anywhere https://github.com/ex-aws/ex_aws_s3/blob/10c0ff5bd6af3644759ecdd7f3f15a9de35a8a4f/lib/ex_aws/s3/parsers.ex#L5

So, as I see, there is two options

  1. Delete current implementation of parse_complete_multipart_upload, because it does nothing rn. And rename parse_upload to parse_complete_multipart_upload
  2. Pass &Parsers.parse_upload/1 as parser here https://github.com/ex-aws/ex_aws_s3/blob/10c0ff5bd6af3644759ecdd7f3f15a9de35a8a4f/lib/ex_aws/s3.ex#L938

I will a PR with solution, that you consider right I, personally, prefer first solution.

khaskelberg avatar Sep 25 '19 17:09 khaskelberg

I can confirm this.

ex_aws      -> 2.1.4
ex_aws_s3 -> 2.0.2

Is there a plan to go either of the directions mentioned by @khaskelberg?

Ivor avatar Aug 24 '20 11:08 Ivor

This is now hopefully resolved by #217

bernardd avatar Sep 25 '23 06:09 bernardd