Convert Access Log Generator to use Python logging#473
Convert Access Log Generator to use Python logging#473mbidewell wants to merge 6 commits intodjango:mainfrom
Conversation
|
Any update? |
|
@sikmir Needs a rebase and a review, but looks along the right lines. |
mbidewell
force-pushed
the
access-logging-handler
branch
from
f7e1f21 to
c741811
Compare
|
Sorry got busy and missed this. branch should be rebased |
bigfootjon
left a comment
bigfootjon
left a comment
There was a problem hiding this comment.
Just one question, otherwise this lgtm
daphne/access.py
Outdated
| else: | ||
| logger.addHandler(logging.NullHandler) |
There was a problem hiding this comment.
I'm not super familiar with how the logging module works, but why are these lines needed? What happens if they are removed?
There was a problem hiding this comment.
My understanding is that the access log would be written to the default handler (likely console). If I am understanding the cli correctly, the access log is not written unless explicitly enabled,
There was a problem hiding this comment.
Shouldn't we just set propagate = False unconditionally then? (I totally might be misunderstanding how this all works, please bear with me)
There was a problem hiding this comment.
I'll need to dig in after I recreate a testing environment. Looks like some other tweaks may be needed
There was a problem hiding this comment.
@mbidewell did you have a chance to dig into this?
|
Nice initiative, any updates? |
|
I believe I have addressed all feedback. Waiting for merge |
5 participants
Commits to move forward #116. Convert the access log to use the python logging framework. Also pass data as named components to make it easier for JSON formatting