Skip to content
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

Proxito middleware: reset to original urlconf after request #7137

Merged
merged 5 commits into from
May 28, 2020

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented May 28, 2020

This also fixes a flaky test.
We hit this before proxito too

def process_response(self, request, response):
# Reset URLconf for this thread
# to the original one.
set_urlconf(None)
return response

@stsewd stsewd requested review from ericholscher and a team May 28, 2020 02:01
@stsewd
Copy link
Member Author

stsewd commented May 28, 2020

Ok... it doesn't fail on my computer anymore, but it still fails on travis 🙃

Comment on lines +203 to +204
# Reset URLconf for this thread
# to the original one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not put the comment in just one line? 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just like short lines to have more space in my monitor 🤷‍♂️

@ericholscher
Copy link
Member

@stsewd yea, the original test works for me locally, but fails on travis. Really not sure what's going on, but the request-based URLConf stuff is definitely buggy.

@stsewd
Copy link
Member Author

stsewd commented May 28, 2020

@ericholscher tests should be passing now, to replicate locally I had to run the tests a lot of times till I got an error, then trying with/without my fix several times to see if failed or not...

@stsewd stsewd merged commit 5a6fbd7 into master May 28, 2020
@stsewd stsewd deleted the fix-flaky-test branch May 28, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants