-
-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add extend_query method #1128
Add extend_query method #1128
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1128 +/- ##
==========================================
+ Coverage 95.15% 95.20% +0.04%
==========================================
Files 30 30
Lines 4646 4709 +63
Branches 411 417 +6
==========================================
+ Hits 4421 4483 +62
- Misses 199 200 +1
Partials 26 26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Verified this works as expected in diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py
index 6eaba561b..6e0f76a2e 100644
--- a/aiohttp/client_reqrep.py
+++ b/aiohttp/client_reqrep.py
@@ -229,10 +229,7 @@ class ClientRequest:
# assert session is not None
self._session = cast("ClientSession", session)
if params:
- q = MultiDict(url.query)
- url2 = url.with_query(params)
- q.extend(url2.query)
- url = url.with_query(q)
+ url = url.extend_query(params)
self.original_url = url
self.url = url.with_fragment(None)
self.method = method.upper() |
Doing a bit of production testing before merge. If that all goes well, I'll bump the version, do a release and than open a PR to aiohttp to replace the hand rolled method |
In turns out I didn't need to split update_query so I can split this PR up a bit |
What do these changes do?
Adds a new method called
extend_query
that will add additional keys instead of replacing them likeupdate_query
. The goal is to replace the hand rolled version inaiohttp
Are there changes in behavior for the user?
no
Related issue number
#1127