Merge "Stop passing attr to keystoneclient when there's no filter_value"

2 files changed, 19 insertions, 9 deletions

diff --git a/swiftclient/client.py b/swiftclient/client.py
index bd41ec1..959ed8f 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -388,7 +388,7 @@ def get_auth_keystone(auth_url, user, key, os_options, **kwargs):
     insecure = kwargs.get('insecure', False)
     timeout = kwargs.get('timeout', None)
     auth_version = kwargs.get('auth_version', '2.0')
-    debug = logger.isEnabledFor(logging.DEBUG) and True or False
+    debug = logger.isEnabledFor(logging.DEBUG)
 
     ksclient, exceptions = _import_keystone_client(auth_version)
 
@@ -419,11 +419,14 @@ def get_auth_keystone(auth_url, user, key, os_options, **kwargs):
     service_type = os_options.get('service_type') or 'object-store'
     endpoint_type = os_options.get('endpoint_type') or 'publicURL'
     try:
+        filter_kwargs = {}
+        if os_options.get('region_name'):
+            filter_kwargs['attr'] = 'region'
+            filter_kwargs['filter_value'] = os_options['region_name']
         endpoint = _ksclient.service_catalog.url_for(
-            attr='region',
-            filter_value=os_options.get('region_name'),
             service_type=service_type,
-            endpoint_type=endpoint_type)
+            endpoint_type=endpoint_type,
+            **filter_kwargs)
     except exceptions.EndpointNotFound:
         raise ClientException('Endpoint for %s not found - '
                               'have you specified a region?' % service_type)
diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py
index cac4da2..f962c63 100644
--- a/tests/unit/test_shell.py
+++ b/tests/unit/test_shell.py
@@ -1450,6 +1450,7 @@ class TestKeystoneOptions(MockHttpTest):
             self.assertTrue(flag in actual_args)
             self.assertTrue(actual_args[flag])
 
+        check_attr = True
         # check args passed to ServiceCatalog.url_for() method
         self.assertEqual(len(fake_ks.client.service_catalog.calls), 1)
         actual_args = fake_ks.client.service_catalog.calls[0]
@@ -1458,15 +1459,21 @@ class TestKeystoneOptions(MockHttpTest):
             key = key.replace('-', '_')
             if key == 'region_name':
                 key = 'filter_value'
+                if expected is None:
+                    check_attr = False
+                    self.assertNotIn(key, actual_args)
+                    self.assertNotIn('attr', actual_args)
+                    continue
             self.assertIn(key, actual_args)
             self.assertEqual(expected, actual_args[key],
                              'Expected %s for key %s, found %s'
                              % (expected, key, actual_args[key]))
-        key, v = 'attr', 'region'
-        self.assertIn(key, actual_args)
-        self.assertEqual(v, actual_args[key],
-                         'Expected %s for key %s, found %s'
-                         % (v, key, actual_args[key]))
+        if check_attr:
+            key, v = 'attr', 'region'
+            self.assertIn(key, actual_args)
+            self.assertEqual(v, actual_args[key],
+                             'Expected %s for key %s, found %s'
+                             % (v, key, actual_args[key]))
 
     def _test_options(self, opts, os_opts, flags=None, no_auth=False):
         # repeat test for different commands using env and command line options