-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Allow caching of middleware construction #10890
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
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
aiohttp/client_reqrep.py
Outdated
response_params: _ResponseParams, | ||
timeout: ClientTimeout, | ||
params: Query = None, | ||
headers: Optional[LooseHeaders] = None, | ||
skip_auto_headers: Optional[Iterable[str]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just thinking through future maintainability of this class. Should we put in a **kwargs: Any
and not use it, so that any subclass is required to include that and pass it through the super() call? That way we should be pretty safe to add parameters in future without worrying about breaking any subclasses.
@bdraco Does the proxy code have any impact on middlewares? Seems to be a ClientRequest created here with a lot less arguments, just wondering if there's any issue here: Line 1364 in 180b5ab
Otherwise, I guess that's why all those parameters have defaults currently, in which case we'll need to do the same for these 2 as well. |
❌ 11 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Feels to me like there should be a separate ClientProxyRequest or something. I think if we separated them, it'd remove a bunch of asserts and make static typing a lot cleaner. |
CodSpeed Performance ReportMerging #10890 will not alter performanceComparing Summary
|
Just putting this here so we don't forget about it. Feel free to pick it up.