-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Added auth and standard logging with templating and file rolling #52
Conversation
I am ready with a pull request to use |
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.
If you could rebase and fix the conflicts and fix the couple of comments I've made I'm happy to get this merged! Thanks for your work so far :)
@MisterWil Do you have any time to complete this? |
Sorry for the delay. This just got lost in the shuffle. I believe I've resolved all of the merge conflicts, added the changes you requested, and all tests pass. :-) ETA: Shoot, I just noticed it looks like a bungled the merge somehow and blew up the commit log. I was trying to fix some problems with my environment at the same time and I have no idea what happened. |
Alright I managed to fix it. I'm not sure what happened but editing this PR to point to a different branch and then pointing it back to master fixed the issue. Please let me know if anything else needs to be done before this can be pulled in - sorry again for the delay. |
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.
Added a few comments below, looks like there are a few semi-colons instead of colons in error messages, unless you had intended this?
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.
Looking good, let's get this merged and take it for a spin!
Description
Created a logging package to handle logging standard, authentication, and request logs with templating. This package is roughly the same as the stdlib log package but with some additional handling for customizing the template of the logger output.
Added configuration options for outputting logs to a file with support for log rolling using the lumberjack library.
Motivation and Context
Done to meet the requirements for issue #45.
How Has This Been Tested?
Despite many files being impacted, most of those changes were to just replace existing Printf/Fatal logging lines to run through the new logging package.
Testing via the standard unit tests provided, modifying only the logging_handler test to point to the new constants and changed constructor.
Checklist: