Update req

[ckoch-cars]

Test the :legacy_headers_as_lists config.

[ckoch-cars]

Hi @wojtekmach, I am trying to test out the updated req and the config option :legacy_headers_as_lists.

Using Application.put_env/3 isn't working. Any suggestions on how we can test this config?

[wojtekmach]

The legacy config is compile-time so unfortunately you won't be able to test both when it is enabled and when it is disabled. My suggestion is to not enable it on your projects. The transition is rough but I think ultimately worth it, I think the map semantics are much nicer here. However, if headers being maps are a deal breaker, i.e. not just a matter of updating the code but it breaks contracts with things outside of your control, please let us know.

[wojtekmach]

FYI some servers, even thought it is against spec, requires headers to be in particular case (we now downcase) and particular order (which we no longer have with maps) so I am planning to leave an escape hatch open and it might be to allow keeping headers as lists to keep order. I haven't thought this through but it might be that instead of headers be a %{String.t() => [String.t()]} map they would be an Enumerable.t({String.t(), [String.t()]) which would still be a breaking change (since the value is a list.)

[ckoch-cars]

The legacy config is compile-time so unfortunately you won't be able to test both when it is enabled and when it is disabled. My suggestion is to not enable it on your projects. The transition is rough but I think ultimately worth it, I think the map semantics are much nicer here. However, if headers being maps are a deal breaker, i.e. not just a matter of updating the code but it breaks contracts with things outside of your control, please let us know.

I'm trying to force the change in config.exs, but that config test is still failing.

I agree about updating the headers everywhere. Hopefully we don't have any services downstream that care about header casing, though there's probably an AWS service that does. 😀

[ckoch-cars]

The legacy config is compile-time so unfortunately you won't be able to test both when it is enabled and when it is disabled. My suggestion is to not enable it on your projects. The transition is rough but I think ultimately worth it, I think the map semantics are much nicer here. However, if headers being maps are a deal breaker, i.e. not just a matter of updating the code but it breaks contracts with things outside of your control, please let us know.

I'm trying to force the change in config.exs, but that config test is still failing.

I agree about updating the headers everywhere. Hopefully we don't have any services downstream that care about header casing, though there's probably an AWS service that does. 😀

UPDATE: I cleaned req and then recompiled and that got the update applied.

[wojtekmach]

Yeah I forgot to mention you need to recompile deps after changing compile-time configs. I believe on Elixir 1.16 that will no longer be necessary, that is, the compiler will know it needs to recompile.