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

Fix Req example HttpClient implementation for Req 0.4+ #1026

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

reisub
Copy link
Contributor

@reisub reisub commented Jan 3, 2024

This builds on work in #1015 adding a bit of code to adjust the header format to the expected list of 2-element tuples.

Context

Req 0.4+ changed the headers format to be a map where the key is a string and the value is a list of strings, e.g.:

%{
  "content-length" => ["12345"]
}

This req change makes the current example fail whenever you try to perform a streaming operation because the code in ExAws.S3.Download assumes a single value:

defp get_file_size(bucket, path, config) do
  %{headers: headers} = ExAws.S3.head_object(bucket, path) |> ExAws.request!(config)

  headers
  |> Enum.find(fn {k, _} -> String.downcase(k) == "content-length" end)
  |> elem(1)
  |> String.to_integer() # <- fails here because it gets a list
end

With the proposed change, the example implementation should handle both new and old Req versions; though I've only tested with a recent one.

This builds on work in ex-aws#1015 adding a bit of code to adjust the header
format to the expected list of 2-element tuples.

Req 0.4+ changed the header format to be a map where the key is a string
and the value is a list of strings, e.g.:

```elixir
%{
  "content-length" => ["12345"]
}
```

This change makes the current example fail whenever you try to perform a
streaming operation because this code in `ExAws.S3.Download` assumes a
single value:

```elixir
defp get_file_size(bucket, path, config) do
  %{headers: headers} = ExAws.S3.head_object(bucket, path) |> ExAws.request!(config)

  headers
  |> Enum.find(fn {k, _} -> String.downcase(k) == "content-length" end)
  |> elem(1)
  |> String.to_integer()
end
```

With the proposed change, the example implementation should handle both new and old
Req versions; though I've only tested with a recent one.
@bernardd
Copy link
Contributor

Thanks @reisub!

@bernardd bernardd merged commit 84db3d0 into ex-aws:main Jan 11, 2024
13 checks passed
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

Successfully merging this pull request may close these issues.

2 participants