-
-
Notifications
You must be signed in to change notification settings - Fork 947
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(testing): add raw_path
to ASGI scope
#2331
base: master
Are you sure you want to change the base?
Conversation
I have an issue with mypy, dunno if it's only me. It complains about line 680 in |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2331 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 7726 7727 +1
Branches 1071 1071
=========================================
+ Hits 7726 7727 +1 ☔ View full report in Codecov by Sentry. |
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 this initiative @aarcex3!
However, it seems there is a misunderstanding regarding the actual implementation.
In ASGI, raw path is presented as the raw_path
key inside scope
. The value is a bytestring path, excluding any query string.
Furthermore, I think this improvement warrants a newsfragment, let's add one.
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.
It seems that some other gates are failing too, not just Mypy.
raw_path
to ASGI scope
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 this improvement, it seems to function as intended now!
I have a couple of nitpicks wrt testing (commented inline); and we also need a news fragment for this issue. The file should be named docs/_newsfragments/2262.newandimproved.rst
.
Unfortunately, we have no bandwidth to help shaping up this PR for the impending beta release that we are trying to haste in order to catch up with CPython 3.13, so we'll have to postpone this PR to Falcon 4.1. No specific action is required from you wrt this point, this is just for reference, that we will not be able to merge this PR until 4.0.0 stable is out.
Summary of Changes
Added a 'raw_path' key to the asgi scope in
create_scope
to preserve the path and keep consistency as per #2159Related Issues
Closes #2262
Pull Request Checklist