-
-
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
Implement Logger middleware regex exclude pattern #1723
Conversation
da9c69b
to
749ca1d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1723 +/- ##
==========================================
+ Coverage 53.49% 53.60% +0.10%
==========================================
Files 128 126 -2
Lines 11979 11985 +6
==========================================
+ Hits 6408 6424 +16
+ Misses 5571 5561 -10
Continue to review full report at Codecov.
|
What -- Support use of regex for logger exclusion rules part of actix#1682
b6938de
to
c6511ac
Compare
Codecov Report
@@ Coverage Diff @@
## master #1723 +/- ##
==========================================
+ Coverage 53.51% 53.58% +0.07%
==========================================
Files 125 125
Lines 11987 12001 +14
==========================================
+ Hits 6415 6431 +16
+ Misses 5572 5570 -2
Continue to review full report at Codecov.
|
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.
Looks alright so far. I think that middleware constructors should panic on invalid input so we can either just panic or require that a regex is passed in that we extract the known good pattern from.
I went with panic, to avoid the user having to rely on the regex crate and also avoid incompatibility if the user's regex crate version significantly differs from the one used by Actix. |
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 agree that not forwarding (leaking) the regex error type is a compelling reason to panic instead. Thanks for tackling this.
PR Type
Feature
PR Checklist
Check your PR fulfills the following:
Overview
Support use of regex for logger exclusion rules
part of #1682