Repository: boto/botocore
Pull request #724 (2015-11-19 18:28:52)

Add support for default client config on session
Exposed a set_default_client_config() and get_default_client_config() that will be used as the config for when clients are created. If a config is also passed to create_client() the config will be merged into the default client config and only override default client config values with non-default values provided to the client config. Also added a merge() method on client config to help facilitate the merging of configs.

This is really going to help with the unification of model-driven CLI commands and custom commands in terms of handling arguments that need to be passed to the creation of clients. For example, it will allow me to remove all of the logic that I added to the s3 commands for this CLI PR: https://github.com/aws/aws-cli/pull/1649

cc @jamesls @mtdowling @rayluo @JordonPhillips 

kyleknap: @jamesls I updated the PR to make the merge implementation more general. It required me storing the user provided options and make the signature use ``*args`` and ``**kwargs``. I added some validation as well to help ensure user constructs the config correctly. Let me know if I am missing any checks for these.
jamesls: :shipit:  Looks good, I think `merge()` looks much cleaner now.  Just had one small request for a test.

Patch:
diff --git a/botocore/client.py b/botocore/client.py
index 203e823db..173331550 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -10,7 +10,6 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
-import re
 import copy
 import logging
 
@@ -18,6 +17,7 @@ import botocore.serialize
 import botocore.validate
 from botocore import waiter, xform_name
 from botocore.awsrequest import prepare_request_dict
+from botocore.compat import OrderedDict
 from botocore.endpoint import EndpointCreator, DEFAULT_TIMEOUT
 from botocore.exceptions import ClientError, DataNotFoundError
 from botocore.exceptions import OperationNotPageableError
@@ -612,11 +612,13 @@ class Config(object):
 
     :type connect_timeout: int
     :param connect_timeout: The time in seconds till a timeout exception is
-        thrown when attempting to make a connection.
+        thrown when attempting to make a connection. The default is 60
+        seconds.
 
     :type read_timeout: int
     :param read_timeout: The time in seconds till a timeout exception is
-        thrown when attempting to read from a connection.
+        thrown when attempting to read from a connection. The default is
+        60 seconds.
 
     :type s3: dict
     :param s3: A dictionary of s3 specific configurations.
@@ -635,19 +637,63 @@ class Config(object):
                   * path -- Addressing style is always by path. Endpoints will
                             be addressed as such: s3.amazonaws.com/mybucket
     """
-    def __init__(self, region_name=None, signature_version=None,
-                 user_agent=None, user_agent_extra=None,
-                 connect_timeout=DEFAULT_TIMEOUT,
-                 read_timeout=DEFAULT_TIMEOUT,
-                 s3=None):
-        self.region_name = region_name
-        self.signature_version = signature_version
-        self.user_agent = user_agent
-        self.user_agent_extra = user_agent_extra
-        self.connect_timeout = connect_timeout
-        self.read_timeout = read_timeout
-        self._validate_s3_configuration(s3)
-        self.s3 = s3
+    OPTION_DEFAULTS = OrderedDict([
+        ('region_name', None),
+        ('signature_version', None),
+        ('user_agent', None),
+        ('user_agent_extra', None),
+        ('connect_timeout', DEFAULT_TIMEOUT),
+        ('read_timeout', DEFAULT_TIMEOUT),
+        ('s3', None)
+    ])
+
+    def __init__(self, *args, **kwargs):
+        self._user_provided_options = self._record_user_provided_options(
+            args, kwargs)
+
+        # Merge the user_provided options onto the default options
+        config_vars = copy.copy(self.OPTION_DEFAULTS)
+        config_vars.update(self._user_provided_options)
+
+        # Set the attributes based on the config_vars
+        for key, value in config_vars.items():
+            setattr(self, key, value)
+
+        # Validate the s3 options
+        self._validate_s3_configuration(self.s3)
+
+    def _record_user_provided_options(self, args, kwargs):
+        option_order = list(self.OPTION_DEFAULTS)
+        user_provided_options = {}
+
+        # Iterate through the kwargs passed through to the constructor and
+        # map valid keys to the dictionary
+        for key, value in kwargs.items():
+            if key in self.OPTION_DEFAULTS:
+                user_provided_options[key] = value
+            # The key must exist in the available options
+            else:
+                raise TypeError(
+                    'Got unexpected keyword argument \'%s\'' % key)
+
+        # The number of args should not be longer than the allowed
+        # options
+        if len(args) > len(option_order):
+            raise TypeError(
+                'Takes at most %s arguments (%s given)' % (
+                    len(option_order), len(args)))
+
+        # Iterate through the args passed through to the constructor and map
+        # them to appropriate keys.
+        for i, arg in enumerate(args):
+            # If it a kwarg was specified for the arg, then error out
+            if option_order[i] in user_provided_options:
+                raise TypeError(
+                    'Got multiple values for keyword argument \'%s\'' % (
+                        option_order[i]))
+            user_provided_options[option_order[i]] = arg
+
+        return user_provided_options
 
     def _validate_s3_configuration(self, s3):
         if s3 is not None:
@@ -655,3 +701,26 @@ class Config(object):
             if addressing_style not in ['virtual', 'auto', 'path', None]:
                 raise InvalidS3AddressingStyleError(
                     s3_addressing_style=addressing_style)
+
+    def merge(self, other_config):
+        """Merges the config object with another config object
+
+        This will merge in all non-default values from the provided config
+        and return a new config object
+
+        :type other_config: botocore.client.Config
+        :param other config: Another config object to merge with. The values
+            in the provided config object will take precedence in the merging
+
+        :rtype: botocore.client.Config
+        :returns: A config object built from the merged values of both
+            config objects.
+        """
+        # Make a copy of the current attributes in the config object.
+        config_options = copy.copy(self._user_provided_options)
+
+        # Merge in the user provided options from the other config
+        config_options.update(other_config._user_provided_options)
+
+        # Return a new config object with the merged properties.
+        return Config(**config_options)
diff --git a/botocore/session.py b/botocore/session.py
index 7d6599118..6f91652ec 100644
--- a/botocore/session.py
+++ b/botocore/session.py
@@ -149,6 +149,7 @@ class Session(object):
         self._session_instance_vars = {}
         if profile is not None:
             self._session_instance_vars['profile'] = profile
+        self._client_config = None
         self._components = ComponentLocator()
         self._register_components()
 
@@ -378,6 +379,26 @@ class Session(object):
                 pass
         return self._config
 
+    def get_default_client_config(self):
+        """Retrieves the default config for creating clients
+
+        :rtype: botocore.client.Config
+        :returns: The default client config object when creating clients. If
+            the value is ``None`` then there is no default config object
+            attached to the session.
+        """
+        return self._client_config
+
+    def set_default_client_config(self, client_config):
+        """Sets the default config for creating clients
+
+        :type client_config: botocore.client.Config
+        :param client_config: The default client config object when creating
+            clients. If the value is ``None`` then there is no default config
+            object attached to the session.
+        """
+        self._client_config = client_config
+
     def set_credentials(self, access_key, secret_key, token=None):
         """
         Manually create credentials for this session.  If you would
@@ -727,15 +748,28 @@ class Session(object):
             the client.  Same semantics as aws_access_key_id above.
 
         :type config: botocore.client.Config
-        :param config: Advanced client configuration options. If region_name
+        :param config: Advanced client configuration options. If a value
             is specified in the client config, its value will take precedence
             over environment variables and configuration values, but not over
-            a region_name value passed explicitly to the method.
+            a value passed explicitly to the method. If a default config
+            object is set on the session, the config object used when creating
+            the client will be the result of calling ``merge()`` on the
+            default config with the config provided to this call.
 
         :rtype: botocore.client.BaseClient
         :return: A botocore client instance
 
         """
+        default_client_config = self.get_default_client_config()
+        # If a config is provided and a default config is set, then
+        # use the config resulting from merging the two.
+        if config is not None and default_client_config is not None:
+            config = default_client_config.merge(config)
+        # If a config was not provided then use the default
+        # client config from the session
+        elif default_client_config is not None:
+            config = default_client_config
+
         if region_name is None:
             if config and config.region_name is not None:
                 region_name = config.region_name


Test patch:
diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py
index 7fa997771..29e158b8b 100644
--- a/tests/unit/test_client.py
+++ b/tests/unit/test_client.py
@@ -426,8 +426,8 @@ class TestAutoGeneratedClient(unittest.TestCase):
         creator = self.create_client_creator()
         service_client = creator.create_client(
             'myservice', 'us-west-2', credentials=self.credentials)
-        with self.assertRaisesRegexp(TypeError,
-                                     'only accepts keyword arguments') as e:
+        with self.assertRaisesRegexp(
+                TypeError, 'only accepts keyword arguments'):
             service_client.test_operation('foo')
 
     @mock.patch('botocore.client.RequestSigner.sign')
@@ -472,19 +472,22 @@ class TestAutoGeneratedClient(unittest.TestCase):
             service_client.test_operation(Bar='two')
 
     def test_client_with_custom_both_timeout(self):
-        self.create_client_creator().create_client('myservice', 'us-west-2',
+        self.create_client_creator().create_client(
+            'myservice', 'us-west-2',
             client_config=client.Config(connect_timeout=123, read_timeout=234))
         call_kwargs = self.endpoint_creator.create_endpoint.call_args[1]
         self.assertEqual(call_kwargs['timeout'], (123, 234))
 
     def test_client_with_custom_connect_timeout(self):
-        self.create_client_creator().create_client('myservice', 'us-west-2',
+        self.create_client_creator().create_client(
+            'myservice', 'us-west-2',
             client_config=client.Config(connect_timeout=123))
         call_kwargs = self.endpoint_creator.create_endpoint.call_args[1]
         self.assertEqual(call_kwargs['timeout'], (123, DEFAULT_TIMEOUT))
 
     def test_client_with_custom_read_timeout(self):
-        self.create_client_creator().create_client('myservice', 'us-west-2',
+        self.create_client_creator().create_client(
+            'myservice', 'us-west-2',
             client_config=client.Config(read_timeout=234))
         call_kwargs = self.endpoint_creator.create_endpoint.call_args[1]
         self.assertEqual(call_kwargs['timeout'], (DEFAULT_TIMEOUT, 234))
@@ -1025,9 +1028,8 @@ class TestAutoGeneratedClient(unittest.TestCase):
             client.meta.config.s3['addressing_style'], 'virtual')
 
     def test_client_s3_addressing_style_with_incorrect_style(self):
-        creator = self.create_client_creator()
         with self.assertRaises(InvalidS3AddressingStyleError):
-            client_config=client.Config(s3={'addressing_style': 'foo'})
+            client.Config(s3={'addressing_style': 'foo'})
 
     def test_client_s3_addressing_style_config_overrides_scoped_config(self):
         creator = self.create_client_creator()
@@ -1048,7 +1050,7 @@ class TestAutoGeneratedClient(unittest.TestCase):
             client.meta.events.unregister.call_args_list
         )
 
-    def test_client_s3_addressing_style_default_registers_correctly(self):
+    def test_client_s3_addressing_style_auto_registers_correctly(self):
         event_emitter = mock.Mock()
         creator = self.create_client_creator(event_emitter=event_emitter)
         client = creator.create_client(
@@ -1106,3 +1108,78 @@ class TestAutoGeneratedClient(unittest.TestCase):
                               scoped_config={'ca_bundle': 'mycerts.pem'})
         call_kwargs = self.endpoint_creator.create_endpoint.call_args[1]
         self.assertEqual(call_kwargs['verify'], 'verify.pem')
+
+
+class TestConfig(unittest.TestCase):
+    def test_can_use_args_to_construct(self):
+        config = client.Config(*client.Config.OPTION_DEFAULTS.values())
+        for option, default_value in client.Config.OPTION_DEFAULTS.items():
+            self.assertTrue(hasattr(config, option))
+            self.assertEqual(getattr(config, option), default_value)
+
+    def test_can_use_kwargs_to_construct(self):
+        config = client.Config(**client.Config.OPTION_DEFAULTS)
+        for option, default_value in client.Config.OPTION_DEFAULTS.items():
+            self.assertTrue(hasattr(config, option))
+            self.assertEqual(getattr(config, option), default_value)
+
+    def test_can_use_mix_of_args_and_kwargs(self):
+        config = client.Config('us-east-1', read_timeout=50)
+        self.assertEqual(config.region_name, 'us-east-1')
+        self.assertEqual(config.read_timeout, 50)
+
+    def test_invalid_kwargs(self):
+        with self.assertRaisesRegexp(TypeError, 'Got unexpected keyword'):
+            client.Config(foo='foo')
+
+    def test_pass_invalid_length_of_args(self):
+        with self.assertRaisesRegexp(TypeError, 'Takes at most'):
+            client.Config('foo', *client.Config.OPTION_DEFAULTS.values())
+
+    def test_create_with_multiple_kwargs(self):
+        with self.assertRaisesRegexp(TypeError, 'Got multiple values'):
+            client.Config('us-east-1', region_name='us-east-1')
+
+    def test_merge_returns_new_config_object(self):
+        config = client.Config()
+        other_config = client.Config()
+        new_config = config.merge(other_config)
+        # Check the type is correct
+        self.assertIsInstance(new_config, client.Config)
+        # Make sure the new config is a brand new config object
+        self.assertIsNot(new_config, config)
+        self.assertIsNot(new_config, other_config)
+
+    def test_general_merge_keeps_default_values(self):
+        config = client.Config()
+        other_config = client.Config()
+        config_properties = vars(config)
+        new_config = config.merge(other_config)
+        # Ensure that the values all stayed the same in the new config
+        self.assertEqual(config_properties, vars(new_config))
+
+    def test_merge_overrides_values(self):
+        config = client.Config(region_name='us-east-1')
+        other_config = client.Config(region_name='us-west-2')
+        new_config = config.merge(other_config)
+        self.assertEqual(new_config.region_name, 'us-west-2')
+
+    def test_merge_overrides_values_even_when_using_default(self):
+        config = client.Config(region_name='us-east-1')
+        other_config = client.Config(region_name=None)
+        new_config = config.merge(other_config)
+        self.assertEqual(new_config.region_name, None)
+
+    def test_merge_overrides_values_even_when_using_default_timeout(self):
+        config = client.Config(read_timeout=30)
+        other_config = client.Config(read_timeout=DEFAULT_TIMEOUT)
+        new_config = config.merge(other_config)
+        self.assertEqual(new_config.read_timeout, DEFAULT_TIMEOUT)
+
+    def test_merge_overrides_only_when_user_provided_values(self):
+        config = client.Config(
+            region_name='us-east-1', signature_version='s3v4')
+        other_config = client.Config(region_name='us-west-2')
+        new_config = config.merge(other_config)
+        self.assertEqual(new_config.region_name, 'us-west-2')
+        self.assertEqual(new_config.signature_version, 's3v4')
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index 5bb58e18f..ee82b6af1 100644
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -414,14 +414,47 @@ class TestCreateClient(BaseSessionTest):
 
     @mock.patch('botocore.client.ClientCreator')
     def test_config_passed_to_client_creator(self, client_creator):
-        config = client.Config()
+        # Make sure there is no default set
+        self.assertEqual(self.session.get_default_client_config(), None)
+
+        # The config passed to the client should be the one that is used
+        # in creating the client.
+        config = client.Config(region_name='us-west-2')
         self.session.create_client('sts', config=config)
+        client_creator.return_value.create_client.assert_called_with(
+            mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY,
+            scoped_config=mock.ANY, client_config=config,
+            api_version=mock.ANY)
+
+    @mock.patch('botocore.client.ClientCreator')
+    def test_create_client_with_default_client_config(self, client_creator):
+        config = client.Config()
+        self.session.set_default_client_config(config)
+        self.session.create_client('sts')
 
         client_creator.return_value.create_client.assert_called_with(
             mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY,
             scoped_config=mock.ANY, client_config=config,
             api_version=mock.ANY)
 
+    @mock.patch('botocore.client.ClientCreator')
+    def test_create_client_with_merging_client_configs(self, client_creator):
+        config = client.Config(region_name='us-west-2')
+        other_config = client.Config(region_name='us-east-1')
+        self.session.set_default_client_config(config)
+        self.session.create_client('sts', config=other_config)
+
+        # Grab the client config used in creating the client
+        used_client_config = (
+            client_creator.return_value.create_client.call_args[1][
+                'client_config'])
+        # Check that the client configs were merged
+        self.assertEqual(used_client_config.region_name, 'us-east-1')
+        # Make sure that the client config used is not the default client
+        # config or the one passed in. It should be a new config.
+        self.assertIsNot(used_client_config, config)
+        self.assertIsNot(used_client_config, other_config)
+
     def test_create_client_with_region(self):
         ec2_client = self.session.create_client(
             'ec2', 'us-west-2')
@@ -509,5 +542,11 @@ class TestComponentLocator(unittest.TestCase):
             self.components.get_component('foo')
 
 
-if __name__ == "__main__":
-    unittest.main()
+class TestDefaultClientConfig(BaseSessionTest):
+    def test_new_session_has_no_default_client_config(self):
+        self.assertEqual(self.session.get_default_client_config(), None)
+
+    def test_set_and_get_client_config(self):
+        client_config = client.Config()
+        self.session.set_default_client_config(client_config)
+        self.assertIs(self.session.get_default_client_config(), client_config)