GetStream / stream-python

Python Client - Build Activity Feeds & Streams with GetStream.io
https://getstream.io
BSD 3-Clause "New" or "Revised" License
141 stars 41 forks source link

Using requests_toolbelt on App Engine to use Stream client #54

Closed mikaelm1 closed 7 years ago

mikaelm1 commented 7 years ago

Hi all,

We're trying to integrate this library into our app, but have been having some issues because our app is hosted on Google App Engine Standard Environment. A similar issue was filed a few months ago and your team sent a PR to requests_toolbelt to fix the error, but the error is still there.

I followed the advice of the OP of issue #43 and changed line 9 in client.py and so far it seems to be working. Would it be possible to add this to the library?

Also, the fix your team merged into requests_toolbelt is no longer in their codebase, and manually adding that fix back in there produces a recursion error.

Steps to reproduce

Calling feed.add_activity(activity_data) without changing line 9 in client.py produces the following error:

ConnectionError: ('Connection aborted.', error(13, 'Permission denied'))

Environment info

Operating System: Ubuntu 14.04 Python version: 2.7 Library version: requests-toolbelt==0.7.0 stream-python==2.3.9

tbarbugli commented 7 years ago

@JelteF can you have a look into this?

JelteF commented 7 years ago

Hi, Mikael

From what I can tell the change I made is still in their repo although slightly modified, https://github.com/sigmavirus24/requests-toolbelt/blob/master/requests_toolbelt/adapters/appengine.py#L193-L194

I think the reason you're still seeing the problem is that they haven't had any releases since July this year, so my change isn't in any official release yet. What you could do to fix this issue for now is install the latest master version of requests instead of the release from pypi. It also seems like this includes a fix for the recursion error you're seeing: https://github.com/sigmavirus24/requests-toolbelt/commit/594b77a04ab3d6d09687dc2420a5366162302908

mikaelm1 commented 7 years ago

Ah, the release date would explain why our local version doesn't contain those changes. @JelteF @tbarbugli Thank you!

Just a heads up, using their latest version resulted in the following error:

    client = stream.connect(config.STREAM_KEY, config.STREAM_SECRET)
        File "/vagrant/lib/stream/__init__.py", line 37, in connect
location=location, base_url=base_url)
    File "/vagrant/lib/stream/client.py", line 66, in __init__
    self.session.mount(self.base_url, HTTPAdapter(max_retries=0))
        File "/vagrant/lib/requests_toolbelt/adapters/appengine.py", line 77, in __init__
super(AppEngineAdapter, self).__init__(*args, **kwargs)
TypeError: __init__() got an unexpected keyword argument 'max_retries'

Removing max_retries from the HTTPAdapter initializer inside client.py got rid of the error.

JelteF commented 7 years ago

Great to hear that you got it working. Regarding the max_retries it seems to be another bug in request_toolbelt again.

I made a pull request that I think will fix the issue: https://github.com/sigmavirus24/requests-toolbelt/pull/174. Could you confirm that it fixes the issue by trying out the "patch-2" branch of my fork: https://github.com/JelteF/requests-toolbelt/tree/patch-2

mikaelm1 commented 7 years ago

I tried with the patch-2 branch and I can confirm that it worked. So I think the solution is to either pull requests_toolbelt (once patch-2 has been merged) directly from their github repo, or replace line 9 in client.py to use the sessions, until they have another release with the new changes in their library. Thanks for the help!!

JelteF commented 7 years ago

Good to hear, than I'll close this issue once the patch is merged.

JelteF commented 7 years ago

It's merged