-
Notifications
You must be signed in to change notification settings - Fork 133
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
fix #373 breaking collecting user information on *not* starlette #385
fix #373 breaking collecting user information on *not* starlette #385
Conversation
The change fixes the check to match the previous code: `if not StarletteRequest and hasattr(request, 'user'):`
Thanks for reporting and PR. The fix looks reasonable. I will double-check it next week as the failing commit was necessary to bypass Starlette's security layer. I want to make sure it works fine in both cases. |
Thanks, the starlette path should still be taken if it's installed so it should be good, but we're running off our fork for the time being so no rush and do whatever validation you think is necessary. I'm slightly surprised there isn't a test for this since I noticed you did add a test for the starlette path so I would have figured there would be the same already, but I guess not and you possibly added the first test for that area 😅 . |
@bxsx / @waltjones any chance this can be reviewed / merged sometime soon? |
@bxsx will need to look. |
@bxsx any chance you have some time soon to review this? |
The currently used version of `setuptools` has a bug, so the version requirements are not properly respected. In the current version, `requests>= 0.12.1` always installs the latest version of the package.
Thanks again for the PR and sorry that it took a bit longer than it should. I approved the changes and added more tests around, so both Django and Starlette should be safe now ;) As this PR contains changes from another unmerged branch (to fix CI builds) it will be merged after #389 approval (within few days). |
This pull request has been linked to Shortcut Story #91323: GH385: v0.16.1 breaks collecting user information on not Starlette apps. |
@terencehonles we have just released the |
Description of the change
The change fixes the check to match the previous code:
if not StarletteRequest and hasattr(request, 'user'):
Type of change
Related issues
Checklists
Development
Code review