-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat: Output full URL path in error messages #785
Conversation
Codecov Report
@@ Coverage Diff @@
## main #785 +/- ##
==========================================
+ Coverage 80.28% 80.29% +0.01%
==========================================
Files 34 34
Lines 3408 3410 +2
Branches 679 679
==========================================
+ Hits 2736 2738 +2
Misses 499 499
Partials 173 173
Continue to review full report at Codecov.
|
Hi @ericboucher, I'm generally in favor of this change, but do you know if there are any security concerns with logging the formatted path, which may contain contextual information (entity IDs, etc.)? |
I was thinking about this. I think it's generally fine since it is not logging URL parameters where you could have a token or sensitive data. An option could be to add a line to obfuscate secrets but I am not sure it's necessary. |
@ericboucher alright 👍. Can you then just add a note to the method's docstring indicating the cases in which it may be desirable to override the method (e.g. when the URL path contains secrets or PII)? That'll render in the docs page. |
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.
@ericboucher lgtm, thanks!
* Output full path in error messages * Update rest.py * Add warning in method * Update rest.py Co-authored-by: Edgar R. M <[email protected]>
Output actual full path in
response_error_message
instead of"{self.path}"