-
Notifications
You must be signed in to change notification settings - Fork 272
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
Convert Access Log Generator to use Python logging #473
base: main
Are you sure you want to change the base?
Conversation
Any update? |
@sikmir Needs a rebase and a review, but looks along the right lines. |
f7e1f21
to
c741811
Compare
Sorry got busy and missed this. branch should be rebased |
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.
Just one question, otherwise this lgtm
daphne/access.py
Outdated
else: | ||
logger.addHandler(logging.NullHandler) |
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need to dig in after I recreate a testing environment. Looks like some other tweaks may be needed
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