Minor changes to version negotiation logic

Address review comments from change set
I0dfa3f7fe0a1e2aaf31d37c46b65cc6c064b5e86

Change-Id: I4f2d34d9348f6f778e14ef346d03dd65f5ef1552

ironicclient/common/http.py

@@ -141,8 +141,7 @@ class VersionNegotiationMixin(object):
         # TODO(TheJulia): We should break this method into several parts,
         # such as a sanity check/error method.
         if (self.api_version_select_state == 'user' and
-                self.os_ironic_api_version != 'latest' and
-                not isinstance(self.os_ironic_api_version, list)):
+                not self._must_negotiate_version()):
             raise exc.UnsupportedVersion(textwrap.fill(
                 _("Requested API version %(req)s is not supported by the "
                   "server, client, or the requested operation is not "
@@ -227,6 +226,10 @@ class VersionNegotiationMixin(object):
         # NOTE(jlvillal): Declared for unit testing purposes
         raise NotImplementedError()
 
+    def _must_negotiate_version(self):
+        return (self.api_version_select_state == 'user' and
+                (self.os_ironic_api_version == 'latest' or
+                 isinstance(self.os_ironic_api_version, list)))
 
 _RETRY_EXCEPTIONS = (exc.Conflict, exc.ServiceUnavailable,
                      exc.ConnectionRefused, kexc.RetriableConnectionFailure)
@@ -371,10 +374,7 @@ class HTTPClient(VersionNegotiationMixin):
         """
         # NOTE(TheJulia): self.os_ironic_api_version is reset in
         # the self.negotiate_version() call if negotiation occurs.
-        if (self.os_ironic_api_version and
-                self.api_version_select_state == 'user' and
-                (self.os_ironic_api_version == 'latest' or
-                 isinstance(self.os_ironic_api_version, list))):
+        if self.os_ironic_api_version and self._must_negotiate_version():
             self.negotiate_version(self.session, None)
 
         # Copy the kwargs so we can reuse the original in case of redirects
@@ -587,10 +587,7 @@ class SessionClient(VersionNegotiationMixin, adapter.LegacyJsonAdapter):
 
         # NOTE(TheJulia): self.os_ironic_api_version is reset in
         # the self.negotiate_version() call if negotiation occurs.
-        if (self.os_ironic_api_version and
-                self.api_version_select_state == 'user' and
-                (self.os_ironic_api_version == 'latest' or
-                 isinstance(self.os_ironic_api_version, list))):
+        if self.os_ironic_api_version and self._must_negotiate_version():
             self.negotiate_version(self.session, None)
 
         kwargs.setdefault('user_agent', USER_AGENT)

ironicclient/tests/unit/common/test_http.py

@@ -231,7 +231,7 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
     def test_negotiate_version_server_user_list(
             self, mock_pvh, mock_msr, mock_save_data):
         # have to retry with simple get
-        mock_pvh.side_effect = iter([(None, None), ('1.1', '1.26')])
+        mock_pvh.side_effect = [(None, None), ('1.1', '1.26')]
         mock_conn = mock.MagicMock()
         self.test_object.api_version_select_state = 'user'
         self.test_object.os_ironic_api_version = ['1.1', '1.6', '1.25',

ironicclient/v1/client.py

@@ -49,7 +49,7 @@ class Client(object):
                 if kwargs['os_ironic_api_version'] == 'latest':
                     raise ValueError(
                         "Invalid configuration defined. "
-                        "The os_ironic_api_versioncan not be set "
+                        "The os_ironic_api_version can not be set "
                         "to 'latest' while allow_api_version_downgrade "
                         "is set.")
                 # NOTE(dtantsur): here we allow the HTTP client to negotiate a