Allows flask.jsonify to return status codes and headers #16
Conversation
| elif any(x is not sentinel for x in (code, headers)): | ||
| # A response can be passed into `make_response` and it will set | ||
| # the key appropriately | ||
| response = make_response(response, code, headers) |
There was a problem hiding this comment.
@eligundry - let's just make those None's.
Because of this:
In [14]: with app.app_context():
make_response("asd", object(), {})
....:
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
AttributeError: 'object' object has no attribute 'encode'Looked into how make_response works and None is not like a special value that clears out status codes or headers, so we can safely use that. That was my original fear and why I recommended a sentinel. My bad, broski!
There was a problem hiding this comment.
Also, we should add a test that doesn't return a tuple, and just returns a Flask Response, because that would have totally caught this bug.
There was a problem hiding this comment.
I fixed this and removed the sentinel value. Also added a test with a normal jsonify.
|
@eligundry - fails across the board for Python2.6. Apparently, |
|
@hoatle let me know if you want this squashed. |
|
@eligundry thank you for your PR. It looks good to me. Btw, have you tried this? http://flask-classful.teracy.org/#adding-resource-representations-get-real-classy-and-put-on-a-top-hat and its tests are here: https://github.com/teracyhq/flask-classful/blob/develop/test_classful/test_representations.py I recommend using representations, however, we should also definitely support this case with your PR. Could you help to squash? After that, I'll merge then. Thank you again. |
eligundry
force-pushed
the
fix-custom-status-codes
branch
from
20237fc to
6bdf0b6
Compare
|
|
||
| def test_jsonify_post_custom_status_code(): | ||
| resp = client.post('/jsonify') | ||
| eq_(resp.status_code, 201) |
There was a problem hiding this comment.
make sure to check the resp.data for all the tests
eligundry
force-pushed
the
fix-custom-status-codes
branch
from
6bdf0b6 to
bd620c0
Compare
Allows flask.jsonify to return status codes and headers Allows flask.jsonify to return status codes and headers Allows flask.jsonify to return status codes and headers Allows flask.jsonify to return status codes and headers Removes sentinel values from proxy response
eligundry
force-pushed
the
fix-custom-status-codes
branch
from
bd620c0 to
3f637de
Compare
|
@eligundry - Python 3.x tests are failing across the board 😦. Super odd... @hoatle - We haven't yet tried representations. I saw those when we found this problem (looking through the source), but didn't start swapping to them. We'll probably do that for our next project. That said, this was definitely a regression so needs a fix either way. Nice part about representations, we can support XML responses for our API for masochists! |
|
@hjc1710 @eligundry for the failed test, please use: |
|
@hoatle Thanks! |
|
I've updated the PR to fix failing tests and updated the commit message here at #20 |
|
@eligundry @hjc1710 thank you very much for you help, Flask-Classful v0.10.0 was released https://pypi.python.org/pypi/Flask-Classful |
|
Awesome! Thanks so much @hoatle! |
4 participants
Recently, we switched from Flask Classy to Flask Classful and discovered that all our routes using
flask.jsonifywith custom headers and status codes weren't working. For example:would return the proper JSON response but would be missing the headers and status code. Currently, we've had to change all those routes to:
This is a definite regression from Flask Classy and this PR remedies that.
Shoutout to @hjc1710 for pair programming this with me.