Cookie extractor from Set-Cookie response.#184
Cookie extractor from Set-Cookie response.#184danielatdattrixdotcom wants to merge 8 commits intosvanoort:masterfrom
Conversation
|
Can one of the admins verify this patch? |
| class CookieExtractor(AbstractExtractor): | ||
| """ Extractor that pulls out a named cookie """ | ||
| extractor_type = 'cookie' | ||
| is_header_extractor = True |
There was a problem hiding this comment.
Excellent! Glad you picked up and added this.
|
@danielatdattrixdotcom Thank you, this is a feature that I know is in fairly high demand. I've added a couple comments (small changes) before this is ready to be merged. It also needs some sort of small unit test in test_validators.py. This weekend I'm going to play with it a bit more and look more closely at the format of output (it might make sense to add a few options). You get kudos by the way for looking closely at how the other extractors are implemented and following that (and adding documentation as well)! |
This reverts commit 11b39f2. Why: breaks URL handling because it applies to the scheme (http:// etc)
… value from the list of extracted named cookies.
|
@svanoort Let me know on the args/options to extractors in the YAML/JSON files and your thoughts on that and I'll make the updates and tests. |
|
Oh no, I had write the same feature code, but @danielatdattrixdotcom 's documents is nice. |
| @@ -1,5 +1,6 @@ | |||
| import logging | |||
| import json | |||
| import Cookie | |||
There was a problem hiding this comment.
Fair, but we need to do the following for python 3 compatibility, since the library changed in Py3 😢 https://docs.python.org/2/library/cookie.html
if 'sys' is not in imports, add it:
PYTHON_MAJOR_VERSION = sys.version_info[0]
if PYTHON_MAJOR_VERSION > 2:
import http.cookies as Cookie
else:
import CookieThere was a problem hiding this comment.
Even easier, just do this:
from .six.moves import http_cookies as Cookie
I'm testing this patch with the above and it works perfectly well on both Py2 and Py3:
$ python2 $(which pyresttest) http://localhost:8000 test/api.yml
Test Group Default SUCCEEDED: : 5/5 Tests Passed!
$ python3 $(which pyresttest) http://localhost:8000 test/api.yml
Test Group Default SUCCEEDED: : 5/5 Tests Passed!
|
@danielatdattrixdotcom Coming back to this now that the crunch period at work is settling down (spoiler: Jenkins, and specifically work on Pipeline Stage View plugin): I think it's not worth adding options to the extractor right now - I had a notion of eventually allowing use of regexes or other matchers in extraction, but it's not useful yet. We need support for templating with collections (or transformations on collections), and/or some embedded transformation language, if we're going to go down that route. It's planned in the future, but too far off to worry about now. Added one comment about a Python 3 compatibility challenge (just an import change). So, from my point of view, to get to merge, what we need are automated tests, covering a couple key edge cases: cookie doesn't exist, cookie exists with multiple values, normal cookie case. This means a unit test in test_validators.py, and a functional test in functionaltest.py -- the unit test just covers the extractor parsing and behavior (you can use some cookie info taken from a browser session). The functional test needs to work with real HTTP requests; I'd prefer to add cookie-setting to the test Django-tastypie app (probably easiest done with the sessions middleware option), but if that proves hard we can just invoke a public API or site, as long as the request is trivial. I've been using GitHub for this in the past. |
I needed this for working with an API that uses Django auth/sessions to work with CSRF cookie and X-CSRFToken header.