Merge pull request #305 from zezha-msft/socket_timeout

jofriedm-msft · web-flow · commit db4909a31823 · 2017-06-13T17:20:30.000-07:00
Make socket timeout configurable
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -2,8 +2,9 @@
 
 > See [BreakingChanges](BreakingChanges.md) for a detailed list of API breaks.
 
-## Version X.X.X:
- - All: Fixed a bug where SAS tokens were being duplicated on retries
+## Version 0.34.3:
+- All: Made the socket timeout configurable. Increased the default socket timeout to 20 seconds.
+- All: Fixed a bug where SAS tokens were being duplicated on retries
 
 ## Version 0.34.2:
 
diff --git a/azure/storage/_connection.py b/azure/storage/_connection.py
@@ -47,8 +47,8 @@
 }
 
 class _ServiceParameters(object):
-    def __init__(self, service, account_name=None, account_key=None, sas_token=None, 
-                 is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE, 
+    def __init__(self, service, account_name=None, account_key=None, sas_token=None,
+                 is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
                  custom_domain=None):
 
         self.account_name = account_name
@@ -83,36 +83,37 @@ def __init__(self, service, account_name=None, account_key=None, sas_token=None,
                 self.protocol = self.protocol if parsed_url.scheme is '' else parsed_url.scheme
             else:
                 if not self.account_name:
-                    raise ValueError(_ERROR_STORAGE_MISSING_INFO)         
+                    raise ValueError(_ERROR_STORAGE_MISSING_INFO)
                 self.primary_endpoint = '{}.{}.{}'.format(self.account_name, service, endpoint_suffix)
-            
+
             # Setup the secondary endpoint
             if self.account_name:
                 self.secondary_endpoint = '{}-secondary.{}.{}'.format(self.account_name, service, endpoint_suffix)
             else:
                 self.secondary_endpoint = None
 
     @staticmethod
-    def get_service_parameters(service, account_name=None, account_key=None, sas_token=None, is_emulated=None, 
-                 protocol=None, endpoint_suffix=None, custom_domain=None, request_session=None, 
-                 connection_string=None):
+    def get_service_parameters(service, account_name=None, account_key=None, sas_token=None, is_emulated=None,
+                 protocol=None, endpoint_suffix=None, custom_domain=None, request_session=None,
+                 connection_string=None, socket_timeout=None):
         if connection_string:
             params = _ServiceParameters._from_connection_string(connection_string, service)
         elif is_emulated:
             params = _ServiceParameters(service, is_emulated=True)
         elif account_name:
             params = _ServiceParameters(service,
-                                      account_name=account_name, 
-                                      account_key=account_key, 
-                                      sas_token=sas_token, 
-                                      is_emulated=is_emulated, 
-                                      protocol=protocol, 
+                                      account_name=account_name,
+                                      account_key=account_key,
+                                      sas_token=sas_token,
+                                      is_emulated=is_emulated,
+                                      protocol=protocol,
                                       endpoint_suffix=endpoint_suffix,
                                       custom_domain=custom_domain)
         else:
             raise ValueError(_ERROR_STORAGE_MISSING_INFO)
 
         params.request_session = request_session
+        params.socket_timeout = socket_timeout
         return params
 
     @staticmethod
@@ -136,10 +137,10 @@ def _from_connection_string(connection_string, service):
         endpoint = config.get(_CONNECTION_ENDPONTS[service])
 
         return _ServiceParameters(service,
-                                  account_name=account_name, 
-                                  account_key=account_key, 
-                                  sas_token=sas_token, 
-                                  is_emulated=is_emulated, 
-                                  protocol=protocol, 
+                                  account_name=account_name,
+                                  account_key=account_key,
+                                  sas_token=sas_token,
+                                  is_emulated=is_emulated,
+                                  protocol=protocol,
                                   endpoint_suffix=endpoint_suffix,
                                   custom_domain=endpoint)
diff --git a/azure/storage/_constants.py b/azure/storage/_constants.py
@@ -36,8 +36,8 @@
 DEV_ACCOUNT_NAME = 'devstoreaccount1'
 DEV_ACCOUNT_KEY = 'Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=='
 
-# Socket timeout in seconds is 11
-SOCKET_TIMEOUT = 11
+# Socket timeout in seconds
+DEFAULT_SOCKET_TIMEOUT = 20
 
 #Encryption constants
 _ENCRYPTION_PROTOCOL_V1 = '1.0'
diff --git a/azure/storage/blob/appendblobservice.py b/azure/storage/blob/appendblobservice.py
@@ -74,7 +74,7 @@ class AppendBlobService(BaseBlobService):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None, 
                  is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
-                 custom_domain=None, request_session=None, connection_string=None):
+                 custom_domain=None, request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests 
@@ -110,11 +110,14 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format.
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         self.blob_type = _BlobTypes.AppendBlob
         super(AppendBlobService, self).__init__(
             account_name, account_key, sas_token, is_emulated, protocol, endpoint_suffix, 
-            custom_domain, request_session, connection_string)
+            custom_domain, request_session, connection_string, socket_timeout)
 
     def create_blob(self, container_name, blob_name, content_settings=None,
                     metadata=None, lease_id=None,
diff --git a/azure/storage/blob/baseblobservice.py b/azure/storage/blob/baseblobservice.py
@@ -150,7 +150,7 @@ class BaseBlobService(StorageClient):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None, 
                  is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
-                 custom_domain=None, request_session=None, connection_string=None):
+                 custom_domain=None, request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests 
@@ -186,6 +186,9 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         service_params = _ServiceParameters.get_service_parameters(
             'blob',
@@ -197,7 +200,8 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             endpoint_suffix=endpoint_suffix,
             custom_domain=custom_domain,
             request_session=request_session,
-            connection_string=connection_string)
+            connection_string=connection_string,
+            socket_timeout=socket_timeout)
 
         super(BaseBlobService, self).__init__(service_params)
 
diff --git a/azure/storage/blob/blockblobservice.py b/azure/storage/blob/blockblobservice.py
@@ -104,7 +104,7 @@ class BlockBlobService(BaseBlobService):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None,
                  is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
-                 custom_domain=None, request_session=None, connection_string=None):
+                 custom_domain=None, request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests
@@ -140,11 +140,14 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format.
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         self.blob_type = _BlobTypes.BlockBlob
         super(BlockBlobService, self).__init__(
             account_name, account_key, sas_token, is_emulated, protocol, endpoint_suffix,
-            custom_domain, request_session, connection_string)
+            custom_domain, request_session, connection_string, socket_timeout)
 
     def put_block(self, container_name, blob_name, block, block_id,
                   validate_content=False, lease_id=None, timeout=None):
diff --git a/azure/storage/blob/pageblobservice.py b/azure/storage/blob/pageblobservice.py
@@ -88,7 +88,7 @@ class PageBlobService(BaseBlobService):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None, 
                  is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
-                 custom_domain=None, request_session=None, connection_string=None):
+                 custom_domain=None, request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests 
@@ -124,11 +124,14 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format.
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         self.blob_type = _BlobTypes.PageBlob
         super(PageBlobService, self).__init__(
             account_name, account_key, sas_token, is_emulated, protocol, endpoint_suffix, 
-            custom_domain, request_session, connection_string)
+            custom_domain, request_session, connection_string, socket_timeout)
 
     def create_blob(
         self, container_name, blob_name, content_length, content_settings=None,
diff --git a/azure/storage/file/fileservice.py b/azure/storage/file/fileservice.py
@@ -124,7 +124,7 @@ class FileService(StorageClient):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None, 
                  protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE, 
-                 request_session=None, connection_string=None):
+                 request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests 
@@ -149,6 +149,9 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format.
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         service_params = _ServiceParameters.get_service_parameters(
             'file',
@@ -158,7 +161,8 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             protocol=protocol, 
             endpoint_suffix=endpoint_suffix,
             request_session=request_session,
-            connection_string=connection_string)
+            connection_string=connection_string,
+            socket_timeout=socket_timeout)
             
         super(FileService, self).__init__(service_params)
 
diff --git a/azure/storage/queue/queueservice.py b/azure/storage/queue/queueservice.py
@@ -132,7 +132,7 @@ class QueueService(StorageClient):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None, 
                  is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
-                 request_session=None, connection_string=None):
+                 request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests 
@@ -161,6 +161,9 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format.
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         service_params = _ServiceParameters.get_service_parameters(
             'queue',
@@ -171,7 +174,8 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             protocol=protocol, 
             endpoint_suffix=endpoint_suffix,
             request_session=request_session,
-            connection_string=connection_string)
+            connection_string=connection_string,
+            socket_timeout=socket_timeout)
             
         super(QueueService, self).__init__(service_params)
 
diff --git a/azure/storage/storageclient.py b/azure/storage/storageclient.py
@@ -29,7 +29,7 @@
 )
 from .retry import ExponentialRetry
 from ._constants import (
-    SOCKET_TIMEOUT
+    DEFAULT_SOCKET_TIMEOUT
 )
 from ._http import HTTPError
 from ._http.httpclient import _HTTPClient
@@ -113,10 +113,11 @@ def __init__(self, connection_params):
 
         protocol = connection_params.protocol
         request_session = connection_params.request_session or requests.Session()
+        socket_timeout = connection_params.socket_timeout or DEFAULT_SOCKET_TIMEOUT
         self._httpclient = _HTTPClient(
             protocol=protocol,
             session=request_session,
-            timeout=SOCKET_TIMEOUT,
+            timeout=socket_timeout,
         )
 
         self.retry = ExponentialRetry().retry
@@ -126,6 +127,14 @@ def __init__(self, connection_params):
         self.response_callback = None
         self.retry_callback = None
 
+    @property
+    def socket_timeout(self):
+        return self._httpclient.timeout
+
+    @socket_timeout.setter
+    def socket_timeout(self, value):
+        self._httpclient.timeout = value
+
     @property
     def protocol(self):
         return self._httpclient.protocol
diff --git a/azure/storage/table/tableservice.py b/azure/storage/table/tableservice.py
@@ -126,7 +126,7 @@ class TableService(StorageClient):
 
     def __init__(self, account_name=None, account_key=None, sas_token=None, 
                  is_emulated=False, protocol=DEFAULT_PROTOCOL, endpoint_suffix=SERVICE_HOST_BASE,
-                 request_session=None, connection_string=None):
+                 request_session=None, connection_string=None, socket_timeout=None):
         '''
         :param str account_name:
             The storage account name. This is used to authenticate requests 
@@ -155,6 +155,9 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             request session. See
             http://azure.microsoft.com/en-us/documentation/articles/storage-configure-connection-string/
             for the connection string format.
+        :param int socket_timeout:
+            If specified, this will override the default socket timeout. The timeout specified is in seconds.
+            See DEFAULT_SOCKET_TIMEOUT in _constants.py for the default value.
         '''
         service_params = _ServiceParameters.get_service_parameters(
             'table',
@@ -165,7 +168,8 @@ def __init__(self, account_name=None, account_key=None, sas_token=None,
             protocol=protocol, 
             endpoint_suffix=endpoint_suffix,
             request_session=request_session,
-            connection_string=connection_string)
+            connection_string=connection_string,
+            socket_timeout=socket_timeout)
             
         super(TableService, self).__init__(service_params)
 
diff --git a/tests/test_client.py b/tests/test_client.py
@@ -167,6 +167,17 @@ def test_create_service_missing_arguments(self):
 
         # Assert
 
+    def test_create_service_with_socket_timeout(self):
+        # Arrange
+
+        for type in SERVICES.items():
+            # Act
+            service = type[0](self.account_name, self.account_key, socket_timeout=22)
+
+            # Assert
+            self.validate_standard_account_endpoints(service, type[1])
+            self.assertEqual(service.socket_timeout, 22)
+
     #--Connection String Test Cases --------------------------------------------
 
     def test_create_service_with_connection_string_key(self):
diff --git a/tests/test_retry.py b/tests/test_retry.py
@@ -16,7 +16,10 @@
 from io import BytesIO
 from azure.storage import LocationMode
 from azure.storage.blob import BlockBlobService
-from azure.common import AzureHttpError
+from azure.common import (
+    AzureHttpError,
+    AzureException,
+)
 from azure.storage.retry import (
     LinearRetry,
     ExponentialRetry,
@@ -98,6 +101,29 @@ def test_retry_on_timeout(self):
         # The retry will then get a 409 and return false.
         self.assertFalse(created)
 
+    @record
+    def test_retry_on_socket_timeout(self):
+        # Arrange
+        container_name = self.get_resource_name()
+        service = self._create_storage_service(BlockBlobService, self.settings)
+        service.retry = LinearRetry(backoff=0.1, max_attempts=3).retry
+
+        # make the connect timeout reasonable, but packet timeout truly small, to make sure the request always times out
+        service.socket_timeout = (11, 0.000000000001)
+
+        # Act
+        try:
+            service.create_container(container_name)
+            self.fail('The socket timeout should force failure.')
+        except AzureException as e:
+            # Assert
+            # This call should succeed on the server side, but fail on the client side due to socket timeout
+            self.assertTrue('ReadTimeout' in e.message, 'Expected socket timeout but got different exception.')
+        finally:
+            # we must make the timeout normal again to let the delete operation succeed
+            service.socket_timeout = (11, 11)
+            service.delete_container(container_name)
+
     @record
     def test_no_retry(self):
         # Arrange
