-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
update regex in AccessLogger to accept underscore #1226
Conversation
update Unit Tests
Current coverage is 98.28% (diff: 100%)@@ master #1226 diff @@
==========================================
Files 29 29
Lines 6485 6485
Methods 0 0
Messages 0 0
Branches 1087 1087
==========================================
Hits 6374 6374
Misses 59 59
Partials 52 52
|
@@ -269,7 +269,7 @@ class AccessLogger: | |||
""" | |||
|
|||
LOG_FORMAT = '%a %l %u %t "%r" %s %b "%{Referrer}i" "%{User-Agent}i"' | |||
FORMAT_RE = re.compile(r'%(\{([A-Za-z\-]+)\}([ioe])|[atPrsbOD]|Tf?)') | |||
FORMAT_RE = re.compile(r'%(\{([A-Za-z\-_]+)\}([ioe])|[atPrsbOD]|Tf?)') |
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.
maybe numbers should be accepted as well?
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.
agree
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.
Not sure should we split header rules and environment variable
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.
updated to accept numbers, not sure about your last comment though
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.
good to 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.
Please add your name to CONTRIBUTORS.txt
@@ -26,7 +26,7 @@ CHANGES | |||
1.0.3 (XXXX-XX-XX) | |||
------------------ | |||
|
|||
- | |||
- Modify regex in AccessLogger to accept underscore #1225 |
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.
Should be in 1.1.0 section unless you make a PR against 1.0 branch.
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.
Thanks for pointing this out, updated
f6037bf
to
8f55e61
Compare
I am already in CONTRIBUTORS.txt from a previous contribution :) |
update unit tests
8f55e61
to
4cd214b
Compare
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.
Cool!
Will merge after passing CI tests
Thanks! |
What do these changes do?
Update regex in AccessLogger to accept underscore, update Unit Tests
Are there changes in behavior for the user?
no
Related issue number
#1225
Checklist
CONTRIBUTORS.txt
CHANGES.rst
#isuue_number
format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.update Unit Tests