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

Allow redirects through to user #162

Merged
merged 2 commits into from
Jan 13, 2014
Merged

Allow redirects through to user #162

merged 2 commits into from
Jan 13, 2014

Conversation

bbenne10
Copy link
Contributor

For some reason flask.redirect returns a response that is derived from werkzeug.wrappers.Response, rather than flask.Response. Testing with some dummy code proves this problem to be a bit inconsistent:

class X(object):
    def __init__(self):
        pass

class Y(X):
    def __init__(self):
        pass

z = Y()
isinstance(z, X)  #  -> True

This conflicts with what I see with Flask / Flask-Restful:

from flask import Response

#...blahblahblah basic setup
x = redirect(url_for('render_root'))
isinstance(x, Response)  #  -> False

The code on this branch fixes this, but there may be a better way to fix this problem. I'd be open to hearing and possibly implementing any other ideas. Do you guys have any other thoughts?

@bbenne10
Copy link
Contributor Author

Okay - upon further investigation, this issue makes more sense and has a slightly different solution than I initially committed. Because flask.Response is derived from werkzeug.wrappers.Response, we should check werkzeug's Response via isinstance, rather than Flask's, as werkzeug's will not match an isinstance check (though they serve the same purpose for us)

Furthermore, my earlier code tests apples against oranges. I was testing two very distinct scenarios. Please ignore it, but I'll leave it up for posterity's sake.

@lanceingle
Copy link
Contributor

+1 I needed this the other day.

Returning a Redirect object in a Restful.Response inherited class will result in "Response 302 not JSON Serializable". This pull-request fixes that.

Here is a example of the problem

from flask import Flask, redirect
from flask.ext import restful

app = Flask(__name__)
api = restful.Api(app)


class HelloWorld(restful.Resource):
    def get(self):
        return redirect('/')

api.add_resource(HelloWorld, '/api/')


@app.route('/')
def index():
    return "Hello World"

if __name__ == "__main__":
    app.debug = True
    app.run()

// lance

@bbenne10
Copy link
Contributor Author

Does anyone have any thoughts on this?

@feigner
Copy link

feigner commented Jan 13, 2014

+1 I need this in my life. Any idea on PR status? Seems straightforward enough...

@skimbrel
Copy link
Contributor

Looks reasonable to me. I'll use the provided example problem code to write up a quick test for this behavior. Thanks!

skimbrel added a commit that referenced this pull request Jan 13, 2014
Allow redirects through to user
@skimbrel skimbrel merged commit 092af65 into flask-restful:master Jan 13, 2014
@bbenne10
Copy link
Contributor Author

I'm deleting my branch since this is merged. Glad to see it pulled in. Thanks, @skimbrel!

@bbenne10 bbenne10 deleted the ResponseFix branch January 13, 2014 21:05
@feigner
Copy link

feigner commented Jan 13, 2014

Heck yes! Thanks @bbenne10, @skimbrel!

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.

4 participants