watson-developer-cloud / python-sdk

:snake: Client library to use the IBM Watson services in Python and available in pip as watson-developer-cloud
https://pypi.org/project/ibm-watson/
Apache License 2.0
1.46k stars 827 forks source link

Feature Request: Allow specification of connection timeout #327

Closed breathe closed 6 years ago

breathe commented 6 years ago

Would be nice if we could pass a parameter through to the 'requests' library to control connection timeout.

germanattanasio commented 6 years ago

This is a good idea. It will also help if we can specify a proxy.

germanattanasio commented 6 years ago

@breathe I think it will take some time because people are on vacation. Feel free to open a PR if you have the time.

breathe commented 6 years ago

@germanattanasio I can submit a PR -- I'm thinking:

timeout as an additional optional argument to DiscoveryV1.__init__

class DiscoveryV1(WatsonDeveloperCloudService):
    """Client for Discovery service"""

    def __init__(self,
                 version,
                 url=default_url,
                 username=None,
                 password=None,
                 use_vcap_services=True,
                 timeout=None):

And then adding timeout parameter within DiscoveryV1.request method:

        response = requests.request(method=method, url=full_url,
                                    cookies=self.jar, auth=auth,
                                    headers=headers,
                                    params=params, data=data, files=files,
                                    timeout=self.timeout
                                    **kwargs)

Sound about right...?

Would let me get rid of this ugly monkeypatch:

# EVIL: MONKEY PATCH THE REQUESTS LIBRARY TO ADD TIMEOUT TO ALL requests.request() calls
originalRequestFunction = requests.request

def monkeyPatchedRequest(*args, **kwords):
    return originalRequestFunction(*args, **kwords, timeout=(5, 5))

requests.request = monkeyPatchedRequest
# END EVIL
germanattanasio commented 6 years ago

I think that having timeout in the init() method is enough. Should the name be timeout? should we wrap timeout and other parameters like proxies in a dictionary. I'm thinking of something like http_config. 🤔

def __init__(self,
                 version,
                 url=default_url,
                 username=None,
                 password=None,
                 use_vcap_services=True,
                 http_config=None):
breathe commented 6 years ago

http_config as a dictionary might be closer to the right thing. I'll update the PR

ehdsouza commented 6 years ago

@breathe thanks for suggesting this. It has been handled in PR: https://github.com/watson-developer-cloud/python-sdk/pull/340