-
-
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
aiohttp.wep.Application.make_handler access_log_class support #2316
Changes from 11 commits
17364dc
9dcb69d
bff0c8e
07dd227
9a6e28b
bbd6383
8f19815
d825c4b
ced7bad
fc75be3
a9246ad
f0ac3e3
db02580
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -146,3 +146,27 @@ def write_eof(self, chunk=b''): | |
@abstractmethod | ||
def drain(self): | ||
"""Flush the write buffer.""" | ||
|
||
|
||
class AbstractAccessLogger(ABC): | ||
|
||
def __init__(self, logger, log_format): | ||
self.logger = logger | ||
self.log_format = log_format | ||
|
||
@abstractmethod | ||
def _log(self, request, response, time): | ||
""" | ||
|
||
:param request: Request object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't use sphinx formatting in docstrings -- it is a text for readers. |
||
:param response: Response object. | ||
:param float time: Time taken to serve the request. | ||
""" | ||
|
||
def log(self, request, response, time): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to document which objects and types that method accepts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, it will be documented |
||
"""Emit log to logger""" | ||
|
||
try: | ||
self._log(request, response, time) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's drop |
||
except Exception: | ||
self.logger.exception("Error in logging") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ | |
from . import (hdrs, web_exceptions, web_fileresponse, web_middlewares, | ||
web_protocol, web_request, web_response, web_server, | ||
web_urldispatcher, web_ws) | ||
from .abc import AbstractMatchInfo, AbstractRouter | ||
from .abc import AbstractAccessLogger, AbstractMatchInfo, AbstractRouter | ||
from .frozenlist import FrozenList | ||
from .http import HttpVersion # noqa | ||
from .log import access_logger, web_logger | ||
|
@@ -231,6 +231,14 @@ def middlewares(self): | |
return self._middlewares | ||
|
||
def make_handler(self, *, loop=None, **kwargs): | ||
if 'access_log_class' in kwargs: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just add |
||
access_log_class = kwargs['access_log_class'] | ||
|
||
assert issubclass(access_log_class, AbstractAccessLogger), ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
'{} must be subclass of ' | ||
'aiohttp.abc.AbstractAccessLogger'.format(access_log_class) | ||
) | ||
|
||
self._set_loop(loop) | ||
self.freeze() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
aiohttp.wep.Application.make_handler support access_log_class |
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.
log_format
seems to be lost.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.
Yes, but it is exactly how current
AccessLogger
works, as well defaultlog_format
isAccessLogger.LOG_FORMAT
not sure is it needs to be stored asself.log_format
. My idea was that,logger
itself is only one required attribute,log_format
is actually optionalThere 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.
So, if I'm making own
AccessLogger
I should keep in mind, that super requires two arguments, one of which it will set to attributes and one that it will ignore? A bit not friendly behavior. TheAccessLogger
may have overloaded__init__
to acceptlog_format
, but if theAbstractAccessLogger
doesn't uses it in anyway there is no point to keep it there.