-
Notifications
You must be signed in to change notification settings - Fork 809
Add CORS-middleware #300
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
Add CORS-middleware #300
Conversation
why not simply use django-cors-headers ? |
django-cors-headers has no way of determining headers dynamically, this PR aims to solve that. There's more info in the issue I just linked in the PR (forgot that earlier unfortunately) |
This branch needs updating with develop, a quick mention in the documentation, and an up/down vote on merging it. One question I had was if this replaces django-cors-headers or if it just supplements it's behavior? i.e. would you run both at the same time or not? It seems others have need for it, it's optional, tested, and as long as it is a sane implementation it seems ok to add. |
7922bb6
to
24ee096
Compare
Now it's synced with evonove:master (Didn't find any develop?) and I've added some documentation, I only added to the tutorial (where all django-cors-headers references were removed). Does that make sense? (We've been using this approach in prod for several months, since the PR was opened) |
It seems that the changes has not yet been put when using pip,, I also checked my oauth2_provider.middleware whch was download just last week and it has no "CorsMiddleware" class in it |
80a3973
to
f59509f
Compare
Closing PR which at this point is too old to review/merge. If there's interest to revive it please rediscuss in #287. |
Why was this PR killed? This seems to be novel functionality apart from that offered by django-cors-headers. To allow external clients to hit our API I'm going to have to reimplement this code downstream to replicate the functionality; IMO this is very useful. |
@rcmurphy Like it says, because it's too old to review/merge. Feel free to submit a new PR. |
Awesome! Will do. |
As discussed in: #287