req icon indicating copy to clipboard operation
req copied to clipboard

add functionality for base_params

Open mxub opened this issue 2 years ago • 2 comments

I stumbled across an API where I need to carry one specific param the whole time.

Therefore I thought it might be useful to have the base_url functionality for params as well.

The only issue would be if I then have to make a call without the presence of said param.

iex> req = Req.new(base_url: "https://httpbin.org/anything", base_params: [foo: "bar"])
iex> Req.get!(req).body["args"]
%{"foo" => "bar"}
iex> Req.get!(req, params: [bar: "baz"]).body["args"]
%{"bar" => "baz", "foo" => "bar"}
iex> Req.get!("https://httpbin.org/anything", params: [bar: "baz"]).body["args"]
%{"bar" => "baz"}
iex> Req.get!(req, params: [foo: "foo"]).body["args"]
%{"foo" => "foo"}

another caveat is that the put_params check for non-presence is a bit uglier than before ...

mxub avatar Jul 05 '22 17:07 mxub

I thought we'd support this use case by something like this:

Req.new(params: [a: 1]) |> Req.get!(params: [b: 2])

which is similar to how we handle headers, we merge them. But headers are special, currently it is not possible to merge arbitrary options. I don't know if we should allow to merge arbitrary options or we should go with :base_params + :params like you have. Let me think about it. Thanks for the PR!

wojtekmach avatar Jul 05 '22 18:07 wojtekmach

I'm fine with

Req.new(params: [a: 1]) |> Req.get!(params: [b: 2])

I only thought that :base_params are a known behavior because of :base_url. On the other hand: could :base_url be rewitten to use the same style ?

Req.new(url: "https://example.com/") |> Req.get!(url: "foo/bar"). # url now https://example.com/foo/bar
Req.new(url: "https://example.com/") |> Req.get!(url: "https://example2.com/foo/bar"). # url now https://example2.com/foo/bar

mxub avatar Jul 07 '22 07:07 mxub

I'm planning to go with merging :params (no ETA) so I'm going to close this for now. Thank you.

wojtekmach avatar Sep 09 '22 06:09 wojtekmach