-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
access logging refactoring #3795
Comments
as mentioned in #3767, personally I think logging should be accomplished by a simple coroutine, not classes no other options, just: async def request_logger(request, response, start_time):
... If users need to setup logging in some way, they can do that in We can work to make this compatible (include a deprecation warning) with the current behaviour, but I think if we're redesigning the component of aiohttp we should work to make it as simple and easy to use as possible while remaining powerful. |
Well, a simple async function is an option to consider.
Changing logger in |
I guess "simple coroutine" really means "something awaitable", so in fact you could create a class with That way you support the simplest approach but also allow for more unusual/complex scenarios. |
Minor note: A dedicated abstract class allows detecting errors early by |
How often do you want to override access logger? |
sorry, yes I meant
The main use case I'm thinking of is a custom access logger for aiohttp-devtools, this would be better than middleware as it wouldn't involve modifying the app to add middleware. Also to replace this middleware. So basically for tools that are agnostic about the project (but might be configured by setting attributes of |
Middleware replacement is a good point. |
This is the plan.
Yes.
Maybe, not sure. The first step is for supporting access logger object in runners. Application-level API can be considered as the second step. |
Long story short
Now
access_log_class
,access_log
andaccess_log_format
are used by public API to control access logger settings.It is a design mistake, sorry.
The proposal is:
access_log_class
andaccess_log_format
parameterslogging.Logger
value foraccess_log
AbstractAsyncAccessLogger
asaccess_log
parameter.access_log=None
as default, keep the default behavior as is nowweb_protocol.py
toweb_runner.py
web_server
as well.The API uses too much
kwargs
now, backward-compatible changes are possible but need very careful review. All existing public API for access log setup should work in aiohttp 4.0 and can be removed in 5.0The issue requires a lot of work.
I'm happy to review it but have no capacity to do it myself in a few months.
We are looking for a champion :)
See also #3777 and #3767
The text was updated successfully, but these errors were encountered: