feat(django): Pick custom urlconf up from request if any#1308
Merged
sl0thentr0py merged 10 commits intomasterfrom Jan 19, 2022
Merged
feat(django): Pick custom urlconf up from request if any#1308sl0thentr0py merged 10 commits intomasterfrom
sl0thentr0py merged 10 commits intomasterfrom
Conversation
antonpirker
approved these changes
Jan 19, 2022
Contributor
antonpirker
left a comment
There was a problem hiding this comment.
Looks nice. I have just one question.
AbhiPrasad
approved these changes
Jan 19, 2022
|
|
||
| content, status, _headers = client.get("/custom/ok") | ||
| assert status.lower() == "200 ok" | ||
| assert b"".join(content) == b"custom ok" |
Contributor
There was a problem hiding this comment.
why do we need the bytes literal here?
Member
Author
There was a problem hiding this comment.
it's a byte stream or something from werkzeug, not a string. I just followed the other tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Premise
Django middlewares sometimes can override
request.urlconffor the current request which it has a contract for using subsequently as the source of truth for url related operations.As a result, we also need to respect the overwritten
urlconfin our transaction name resolving.Solution
I finally went with an
_after_get_responsesolution. The resolution is repeated code but runs only ifurlconfis overwritten. I wanted to touch as little of existing logic as possible.For reference, I went back and forth several times between doing it in a couple of other places.
_before_get_response- this doesn't work because it runs before all the django middlewaresProblem
One of our customers have a
django-tenantsmulti-tenant setup. They have a dynamically resolvedurlconfand setROOT_URLCONFto an empty string which breaks thisget_resolverin Django.This results in a silent exception when we try to update the transaction name and hence they see
Generic WSGI requestfor all their requests.References