Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

If params=... is passed then replace any existing query parameters, rather than merge them. #2713

Closed
tomchristie opened this issue May 22, 2023 · 5 comments

Comments

@tomchristie
Copy link
Member

Prompted by #2712 (comment).

Currently when the URL contains existing query parameters and the params=... argument is passed, we merge the sets of query parameters...

>>> httpx.Request('GET', 'http://example.com?a=1', params={'b': '2'})
<Request('GET', 'http://example.com?a=1&b=2')>

This seems a bit counter-intuitive to me - I'd probably(?) expect them to be replaced, rather than merged.

Is it worth us changing this behaviour?

Implemented here...

self.url = self.url.copy_merge_params(params=params)

@Tunglies
Copy link

How about giving a keyword and decide replace or merge, likes httpx.Request('GET', 'http://example.com?a=1', params={'b': '2'}, replace=True)

@tomchristie
Copy link
Member Author

Hrrmmm...

We can do this being explicit:

url = httpx.URL('http://example.com?a=1').copy_with(params=...)  # Replaces params
url = httpx.URL('http://example.com?a=1').copy_merge_params(...)  # Merges params
r = httx.Request('GET', url)

@michaeloliverx
Copy link
Member

would this include replacing query params that were passed in the Client(params={}) instantiation?

@zanieb
Copy link
Contributor

zanieb commented Jun 14, 2023

Hm, I think I'd expect merge with the ability to "remove" parameters with {'key': None} but I'm not sure what use-cases there are for providing a URL with existing parameters. It seems like why you would end up in this situation is important for determining the expected behavior.

@tomchristie
Copy link
Member Author

@tomchristie - I'm nudging this issue into a discussion. If we reach a design consensus then we can escalate it into an issue.

@encode encode locked and limited conversation to collaborators Jun 14, 2023
@tomchristie tomchristie converted this issue into discussion #2743 Jun 14, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants