-
Notifications
You must be signed in to change notification settings - Fork 18
#16 use the request path in the exception #17
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
#16 use the request path in the exception #17
Conversation
Good solution! Want to update the accompanying text to something more about the "path" than the "request handler"? |
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.
The feedback from above, but now in review format so I can hit the "Request changes" button 😄
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.
Small change because the text seems to be broken up upon testing
adafruit_wsgi/wsgi_app.py
Outdated
"Proper HTTP response return not given for request handler '{}'".format( | ||
route["func"].__name__ | ||
) | ||
f"Proper HTTP response return not given for request handler for path \ |
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 checked this out, and it seems this will be raised and catch the tabs as follows:
RuntimeError: Proper HTTP response return not given for request handler for path 'some/path/'
Try this out, I think this should both appease pylint
and keep the words together:
"Proper HTTP response return not given for request handler for path "
f"'{request.path}'"
Note the space after path
in the first line!
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.
That should be better there. It didn't need to be f-stringed, as it is already typed as a string, so a simple concatenation should be good enough
adafruit_wsgi/wsgi_app.py
Outdated
route["func"].__name__ | ||
) | ||
"Proper HTTP response not returned by request handler for path " | ||
+ "'{request.path}'" |
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 like the f
is missing for the f-string
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.
Facepalm. Fixed it.
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.
Fantastic! Looks good to me!
Updating https://github.com/adafruit/Adafruit_CircuitPython_WSGI to 1.1.10 from 1.1.9: > Merge pull request adafruit/Adafruit_CircuitPython_WSGI#17 from dannystaple/16/use-request-path-in-exception > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Update .gitignore > Update Black to latest.
Fixes #16.
Output looks like: