Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class. #119

mikelambert · 2015-12-17T10:47:34Z

The goal here is to allow the following code to work:

from requests_toolbelt.adapters import appengine
appengine.monkeypatch()

...and then run all your libraries that have dependencies on requests and sessions buried deep inside, all working properly with URLFetch on Appengine.

The code doesn't work as-is, as it depends on the following change:
urllib3/urllib3#763
...which then would probably need to be released and ported into the requests library as well...not sure how long that takes or what the process is.

This also is tangentially related to the following issues relating to getting requests playing nicely with AppEngine's URLFetch:
https://github.com/kennethreitz/requests/issues/1905
urllib3/urllib3#618

mikelambert · 2015-12-17T10:48:32Z

And once/if this is merged, I'll request that feature/gae be merged into master as well.

Lukasa · 2015-12-17T11:52:49Z

I think this is a reasonable stop-gap solution to this problem. It would be nice if there was a better way to do this though. Class-level variables on the Session, maybe?

mikelambert · 2015-12-17T22:15:41Z

I agree class-level variables on the Session would be better. However, even if we fixed that in requests library, we'd still probably want need to support backwards-compatibility with older versions of the request library, so this code would still be necessary anyway, right? So I viewed that improvement as a non-blocker for this change.

Lukasa · 2015-12-17T22:23:50Z

Yup, it's definitely a non-blocker.

mikelambert · 2015-12-24T15:49:48Z

As requested in urllib3/urllib3#763 , I've moved most of that complexity into this requests-toolbelt patch instead. The urllib3 changes have been merged, so hopefully @sigmavirus24 or @Lukasa can have another look here. Thanks!

theacodes · 2016-01-04T06:58:13Z

requests_toolbelt/adapters/appengine.py

+                redirect=True, assert_same_host=True, timeout=timeout.Timeout.DEFAULT_TIMEOUT,
+                pool_timeout=None, release_conn=None, **response_kw):
+        # Jump through the hoops necessary to call AppEngineManager's API.
+        appengine_manager = gaecontrib.AppEngineManager(


wouldn't it be better to just create this once?

Good idea, fixed. Though really, the way this code gets used, I believe urlopen was only called once per connection anyway. (Though see note in next comment, as that assumption could change.)

seawolf42 · 2016-01-04T10:02:45Z

I'm confused and hoping someone can clarify this for me. I've checked out requests-toolbelt, applied patches 114 and 119, and am referencing that modified version of requests-toolbelt from my project. I am unclear whether I am supposed to also apply 763 to urllib3, I have not yet done this.

I also added the appengine.monkeypatch() call in main.py prior to importing or calling anything else within my application.

When I try to run my project, I get:

'Timeout' object has no attribute 'read'
... most of the stack trace removed ...
.../requests/packages/urllib3/contrib/appengine.py in _get_absolute_timeout
180.        if timeout.read is not timeout.connect: ...

I feel like I'm missing something obvious here... can someone point me at my oversight?

mikelambert · 2016-01-04T12:02:02Z

@seawolf42 Yes, you need to patch 763 into request's version of urllib3.

For background, 763 was merged into urllib3, but requests has its own version of urllib3, and the urllib3 head hasn't been integrated there yet. Once the requests-toolbelt changes go in, I will push harder on getting someone to merge that.

theacodes · 2016-01-04T17:13:05Z

requests_toolbelt/adapters/appengine.py

+    """
+
+    def __init__(self, pool_manager, url):
+        self.appengine_manager = gaecontrib.AppEngineManager(


Could you have the pool_manager construct this and pass it down to each connection?

Great idea, fixed.

theacodes · 2016-01-04T20:30:04Z

@Lukasa, @sigmavirus24 I'm satisfied with this, if you guys want to take a look.

seawolf42 · 2016-01-05T02:21:25Z

@mikelambert, thanks, that did the trick! I can confirm that using just these three patches (114 and 119 on requests-toolbelt and 763 on urllib3 within request) I am able to use python-social-auth on GAE with DjangoAppEngine, at least within the dev server.

Lukasa · 2016-01-05T11:06:12Z

This looks reasonable to me. =)

jonathan-s · 2016-01-20T15:53:31Z

@seawolf42 Did you succeed in production as well not only on the dev server?

sigmavirus24 · 2016-01-20T15:57:46Z

@jonathan-s have you tested this and found problems? If so, please let us know so we can have this in the best shape possible before merging.

jonathan-s · 2016-01-20T16:00:52Z

I haven't tested it, I'll try to make some tests and see what I get.

jonathan-s · 2016-01-20T17:27:13Z

UPDATE: Not an issue.
@sigmavirus24 When I tried it in devapp_server with the following code

from requests_toolbelt.adapters import appengine
from requests_toolbelt.adapters.appengine import AppEngineAdapter
appengine.monkeypatch()

import requests
import webapp2
from settings import JINJA_ENVIRONMENT

class RequestsView(webapp2.RequestHandler):

    def get(self):
        url = 'http://www.example.com'
        session = requests.Session()
        # response = session.mount(url, AppEngineAdapter())
        response = requests.get(url)

        tmp_values = {}
        tmp_values['status'] = response.status_code
        tmp_values['content'] = response.content[:500]

        template = JINJA_ENVIRONMENT.get_template('templates/requests_view.html')
        self.response.write(template.render(tmp_values))

I got this traceback:

 Traceback (most recent call last):
  File "/Users/jonathan/projects/honshuu-console/lib/webapp2.py", line 1535, in __call__
    rv = self.handle_exception(request, response, e)
  File "/Users/jonathan/projects/honshuu-console/lib/webapp2.py", line 1529, in __call__
    rv = self.router.dispatch(request, response)
  File "/Users/jonathan/projects/honshuu-console/lib/webapp2.py", line 1278, in default_dispatcher
    return route.handler_adapter(request, response)
  File "/Users/jonathan/projects/honshuu-console/lib/webapp2.py", line 1102, in __call__
    return handler.dispatch()
  File "/Users/jonathan/projects/honshuu-console/lib/webapp2.py", line 572, in dispatch
    return self.handle_exception(e, self.app.debug)
  File "/Users/jonathan/projects/honshuu-console/lib/webapp2.py", line 570, in dispatch
    return method(*args, **kwargs)
  File "/Users/jonathan/projects/honshuu-console/views/requests_view.py", line 18, in get
    response = requests.get(url)
  File "/Users/jonathan/projects/honshuu-console/lib/requests/api.py", line 67, in get
    return request('get', url, params=params, **kwargs)
  File "/Users/jonathan/projects/honshuu-console/lib/requests/api.py", line 53, in request
    return session.request(method=method, url=url, **kwargs)
  File "/Users/jonathan/projects/honshuu-console/lib/requests/sessions.py", line 468, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/jonathan/projects/honshuu-console/lib/requests/sessions.py", line 576, in send
    r = adapter.send(request, **kwargs)
  File "/Users/jonathan/projects/honshuu-console/lib/requests/adapters.py", line 376, in send
    timeout=timeout
  File "/Users/jonathan/projects/honshuu-console/lib/requests_toolbelt/adapters/appengine.py", line 116, in urlopen
    **response_kw)
  File "/Users/jonathan/projects/honshuu-console/lib/requests/packages/urllib3/contrib/appengine.py", line 105, in urlopen
    deadline=self._get_absolute_timeout(timeout),
  File "/Users/jonathan/projects/honshuu-console/lib/requests/packages/urllib3/contrib/appengine.py", line 180, in _get_absolute_timeout
    if timeout.read is not timeout.connect:
AttributeError: 'Timeout' object has no attribute 'read'

jonathan-s · 2016-01-20T17:31:32Z

Seeing now that it might be because of the internal use of request which uses an earlier version urllib3

seawolf42 · 2016-01-20T17:42:40Z

This is working great in production with both python-social-auth and facebook-sdk packages.

jonathan-s · 2016-01-20T17:48:58Z

I got it working as well. so 👍 for this.

sigmavirus24 · 2016-01-20T18:46:52Z

requests_toolbelt/adapters/appengine.py

+    or if you use libraries that call the requests API directly (ie requests.post),
+    then you may prefer to monkeypatch and auto-configure all newly-constructed Sessions.
+    """
+    # HACK: We should consider modifying urllib3 to support this as an explicit module-level variable.


This comment doesn't make sense.

sigmavirus24 · 2016-01-20T18:54:04Z

Thank you @seawolf42 and @jonathan-s for testing this for us.

mikelambert · 2016-01-23T05:04:30Z

Okay, should have made all the changes you've requested. I believe all failing tests are now unrelated to my changes. And it will be harder to test and verify this code now (like was done earlier up-thread), because there is no 2.10.0 requests library yet. :)

sigmavirus24 · 2016-01-23T22:10:14Z

Could you rebase this please?

…gineAdapter compatibility class. Implement urllib3 wrappers that let requests work on appengine.

mikelambert · 2016-01-24T03:34:22Z

Rebased.

Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class.

sigmavirus24 · 2016-01-24T03:40:33Z

Thanks @mikelambert

mikelambert · 2016-01-24T04:17:50Z

Thanks! Curious, what's the next step here? Push requests to release 2.10.0 with the latest urllib3, and then after you can integrate feature/gae into master?

sigmavirus24 · 2016-01-24T04:59:25Z

Please don't push requests to do anything. Speaking as a maintainer there, we're already aware this is a desirable feature.

Because of the version checking that takes place in the code now, we can actually release the toolbelt when it's ready. I just need to merge (at least) 2 PRs before performing a release here.

mikelambert mentioned this pull request Dec 17, 2015

Add AppEngineAdapter for GAE users #114

Merged

mikelambert force-pushed the feature/gae branch from af81f8a to 85dc8bf Compare December 24, 2015 11:06

theacodes reviewed Jan 4, 2016
View reviewed changes

seawolf42 mentioned this pull request Jan 4, 2016

Google App Engine RuntimeError: error('illegal IP address string passed to inet_pton',) mobolic/facebook-sdk#214

Closed

theacodes reviewed Jan 4, 2016
View reviewed changes

theacodes mentioned this pull request Jan 5, 2016

Make connection return a thread-local instance of http. googleapis/google-cloud-python#1274

Closed

jonathan-s mentioned this pull request Jan 20, 2016

Cut a release to match urllib3-1.14? psf/requests#2969

Closed

sigmavirus24 reviewed Jan 20, 2016
View reviewed changes

sigmavirus24 force-pushed the feature/gae branch from ad79242 to a22395f Compare January 23, 2016 22:10

mikelambert force-pushed the feature/gae branch 3 times, most recently from 5005ea7 to 584501e Compare January 24, 2016 03:23

Adds a monkeypatch() function that ensures all Sessions use the AppEn…

302e570

…gineAdapter compatibility class. Implement urllib3 wrappers that let requests work on appengine.

mikelambert force-pushed the feature/gae branch from 584501e to 302e570 Compare January 24, 2016 03:33

sigmavirus24 added a commit that referenced this pull request Jan 24, 2016

Merge pull request #119 from mikelambert/feature/gae

075af39

Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class.

sigmavirus24 merged commit 075af39 into requests:feature/gae Jan 24, 2016

sigmavirus24 modified the milestone: 0.6.0 Jan 24, 2016

jonathan-s mentioned this pull request Feb 2, 2016

Integration tests and urlfetch #134

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class. #119

Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class. #119

mikelambert commented Dec 17, 2015

mikelambert commented Dec 17, 2015

Lukasa commented Dec 17, 2015

mikelambert commented Dec 17, 2015

Lukasa commented Dec 17, 2015

mikelambert commented Dec 24, 2015

theacodes Jan 4, 2016

mikelambert Jan 4, 2016

seawolf42 commented Jan 4, 2016

mikelambert commented Jan 4, 2016

theacodes Jan 4, 2016

mikelambert Jan 4, 2016

theacodes commented Jan 4, 2016

seawolf42 commented Jan 5, 2016

Lukasa commented Jan 5, 2016

jonathan-s commented Jan 20, 2016

sigmavirus24 commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

seawolf42 commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

sigmavirus24 Jan 20, 2016

sigmavirus24 commented Jan 20, 2016

mikelambert commented Jan 23, 2016

sigmavirus24 commented Jan 23, 2016

mikelambert commented Jan 24, 2016

sigmavirus24 commented Jan 24, 2016

mikelambert commented Jan 24, 2016

sigmavirus24 commented Jan 24, 2016

Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class. #119

Adds a monkeypatch() function that ensures all Sessions use the AppEngineAdapter compatibility class. #119

Conversation

mikelambert commented Dec 17, 2015

mikelambert commented Dec 17, 2015

Lukasa commented Dec 17, 2015

mikelambert commented Dec 17, 2015

Lukasa commented Dec 17, 2015

mikelambert commented Dec 24, 2015

theacodes Jan 4, 2016

Choose a reason for hiding this comment

mikelambert Jan 4, 2016

Choose a reason for hiding this comment

seawolf42 commented Jan 4, 2016

mikelambert commented Jan 4, 2016

theacodes Jan 4, 2016

Choose a reason for hiding this comment

mikelambert Jan 4, 2016

Choose a reason for hiding this comment

theacodes commented Jan 4, 2016

seawolf42 commented Jan 5, 2016

Lukasa commented Jan 5, 2016

jonathan-s commented Jan 20, 2016

sigmavirus24 commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

seawolf42 commented Jan 20, 2016

jonathan-s commented Jan 20, 2016

sigmavirus24 Jan 20, 2016

Choose a reason for hiding this comment

sigmavirus24 commented Jan 20, 2016

mikelambert commented Jan 23, 2016

sigmavirus24 commented Jan 23, 2016

mikelambert commented Jan 24, 2016

sigmavirus24 commented Jan 24, 2016

mikelambert commented Jan 24, 2016

sigmavirus24 commented Jan 24, 2016