From ee79f6166e2f91725d08f17dc2ee59ca6487cd9b Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Sat, 26 May 2018 10:53:33 -0700
Subject: [PATCH] Use openstack.config for config and Session objects

The TimingSession functionality has been merged into keystoneauth. This
allows us to use the CloudRegion object to get the Session directly.

Change-Id: Ib4c9210e681a2d2d9c5fc40de2c3ede1a5003154
---
 lower-constraints.txt                         |  5 +-
 osc_lib/cli/client_config.py                  | 13 ++---
 osc_lib/clientmanager.py                      | 27 ++--------
 osc_lib/command/timing.py                     |  9 ++--
 osc_lib/session.py                            | 50 -------------------
 osc_lib/shell.py                              | 12 +++--
 osc_lib/tests/command/test_timing.py          | 14 ++++--
 osc_lib/tests/test_clientmanager.py           | 21 +++-----
 osc_lib/tests/test_shell.py                   | 37 ++++++--------
 osc_lib/tests/utils/__init__.py               | 34 ++++---------
 .../direct-openstacksdk-535a179f3c645cc0.yaml |  5 ++
 requirements.txt                              |  5 +-
 tox.ini                                       |  1 -
 13 files changed, 65 insertions(+), 168 deletions(-)
 delete mode 100644 osc_lib/session.py
 create mode 100644 releasenotes/notes/direct-openstacksdk-535a179f3c645cc0.yaml

diff --git a/lower-constraints.txt b/lower-constraints.txt
index c015656..4f67f00 100644
--- a/lower-constraints.txt
+++ b/lower-constraints.txt
@@ -31,7 +31,7 @@ Jinja2==2.10
 jmespath==0.9.0
 jsonpatch==1.16
 jsonpointer==1.13
-keystoneauth1==3.4.0
+keystoneauth1==3.7.0
 kombu==4.0.0
 linecache2==1.0.0
 MarkupSafe==1.0
@@ -44,8 +44,7 @@ munch==2.1.0
 netaddr==0.7.18
 netifaces==0.10.4
 openstackdocstheme==1.18.1
-openstacksdk==0.11.2
-os-client-config==1.28.0
+openstacksdk==0.13.0
 os-service-types==1.2.0
 os-testr==1.0.0
 oslo.concurrency==3.25.0
diff --git a/osc_lib/cli/client_config.py b/osc_lib/cli/client_config.py
index 4ae8837..26d4568 100644
--- a/osc_lib/cli/client_config.py
+++ b/osc_lib/cli/client_config.py
@@ -15,15 +15,8 @@
 
 import logging
 
-try:
-    from openstack.config import exceptions as occ_exceptions
-except ImportError:
-    from os_client_config import exceptions as occ_exceptions
-
-try:
-    from openstack.config import loader as config
-except ImportError:
-    from os_client_config import config
+from openstack.config import exceptions as sdk_exceptions
+from openstack.config import loader as config
 
 from oslo_utils import strutils
 import six
@@ -220,7 +213,7 @@ class OSC_Config(config.OpenStackConfig):
                 prompt_options.append(p_opt)
 
         if msgs:
-            raise occ_exceptions.OpenStackConfigException('\n'.join(msgs))
+            raise sdk_exceptions.OpenStackConfigException('\n'.join(msgs))
         else:
             for p_opt in prompt_options:
                 config['auth'][p_opt.dest] = self._pw_callback(p_opt.prompt)
diff --git a/osc_lib/clientmanager.py b/osc_lib/clientmanager.py
index 3c13d01..eff783f 100644
--- a/osc_lib/clientmanager.py
+++ b/osc_lib/clientmanager.py
@@ -19,22 +19,14 @@ import copy
 import logging
 import sys
 
+from openstack.config import loader as config   # noqa
 from openstack import connection
 from oslo_utils import strutils
 import six
 
 from osc_lib.api import auth
 from osc_lib import exceptions
-from osc_lib import session as osc_session
-from osc_lib import version
 
-# NOTE(dtroyer): Attempt an import to detect if the SDK installed is new
-#                enough to not use Profile.  If so, use that.
-try:
-    from openstack.config import loader as config   # noqa
-    profile = None
-except ImportError:
-    from openstack import profile
 
 LOG = logging.getLogger(__name__)
 
@@ -209,22 +201,9 @@ class ClientManager(object):
                 self._cli_options.remote_project_domain_name
             )
 
-        self.session = osc_session.TimingSession(
-            auth=self.auth,
-            verify=self.verify,
-            cert=self.cert,
-            app_name=self._app_name,
-            app_version=self._app_version,
-            additional_user_agent=[('osc-lib', version.version_string)],
-        )
+        self.session = self._cli_options.get_session()
 
-        # Create a common default SDK Connection object if it is the newer
-        # non-Profile style.
-        if profile is None:
-            self.sdk_connection = connection.Connection(
-                config=self._cli_options,
-                session=self.session,
-            )
+        self.sdk_connection = connection.Connection(config=self._cli_options)
 
         self._auth_setup_completed = True
 
diff --git a/osc_lib/command/timing.py b/osc_lib/command/timing.py
index dd2aeb8..3aa618d 100644
--- a/osc_lib/command/timing.py
+++ b/osc_lib/command/timing.py
@@ -27,13 +27,10 @@ class Timing(command.Lister):
 
         results = []
         total = 0.0
-        for url, td in self.app.timing_data:
-            # NOTE(dtroyer): Take the long way here because total_seconds()
-            #                was added in py27.
-            sec = (td.microseconds + (td.seconds + td.days *
-                                      86400) * 1e6) / 1e6
+        for td in self.app.timing_data:
+            sec = td.elapsed.total_seconds()
             total += sec
-            results.append((url, sec))
+            results.append((td.method + ' ' + td.url, sec))
         results.append(('Total', total))
         return (
             column_headers,
diff --git a/osc_lib/session.py b/osc_lib/session.py
deleted file mode 100644
index 9b19fd4..0000000
--- a/osc_lib/session.py
+++ /dev/null
@@ -1,50 +0,0 @@
-#   Licensed under the Apache License, Version 2.0 (the "License"); you may
-#   not use this file except in compliance with the License. You may obtain
-#   a copy of the License at
-#
-#        http://www.apache.org/licenses/LICENSE-2.0
-#
-#   Unless required by applicable law or agreed to in writing, software
-#   distributed under the License is 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.
-#
-
-"""Subclass of keystoneauth1.session"""
-
-from keystoneauth1 import session
-
-
-class TimingSession(session.Session):
-    """A Session that supports collection of timing data per Method URL"""
-
-    def __init__(
-            self,
-            **kwargs
-    ):
-        """Pass through all arguments except timing"""
-        super(TimingSession, self).__init__(**kwargs)
-
-        # times is a list of tuples: ("method url", elapsed_time)
-        self.times = []
-
-    def get_timings(self):
-        return self.times
-
-    def reset_timings(self):
-        self.times = []
-
-    def request(self, url, method, **kwargs):
-        """Wrap the usual request() method with the timers"""
-        resp = super(TimingSession, self).request(url, method, **kwargs)
-        for h in resp.history:
-            self.times.append((
-                "%s %s" % (h.request.method, h.request.url),
-                h.elapsed,
-            ))
-        self.times.append((
-            "%s %s" % (resp.request.method, resp.request.url),
-            resp.elapsed,
-        ))
-        return resp
diff --git a/osc_lib/shell.py b/osc_lib/shell.py
index 7633b93..4ddc51c 100644
--- a/osc_lib/shell.py
+++ b/osc_lib/shell.py
@@ -38,6 +38,7 @@ from osc_lib import exceptions as exc
 from osc_lib.i18n import _
 from osc_lib import logs
 from osc_lib import utils
+from osc_lib import version
 
 osprofiler_profiler = importutils.try_import("osprofiler.profiler")
 
@@ -336,7 +337,7 @@ class OpenStackShell(app.App):
     * super()
     * _final_defaults()
     * OpenStackConfig
-    * get_one_cloud
+    * get_one
     * _load_plugins()
     * _load_commands()
     * ClientManager
@@ -423,7 +424,7 @@ class OpenStackShell(app.App):
 
         # NOTE(dtroyer): Need to do this with validate=False to defer the
         #                auth plugin handling to ClientManager.setup_auth()
-        self.cloud = self.cloud_config.get_one_cloud(
+        self.cloud = self.cloud_config.get_one(
             cloud=self.options.cloud,
             argparse=self.options,
             validate=False,
@@ -462,7 +463,7 @@ class OpenStackShell(app.App):
         )
 
         # NOTE(dtroyer): If auth is not required for a command, skip
-        #                get_one_Cloud()'s validation to avoid loading plugins
+        #                get_one()'s validation to avoid loading plugins
         validate = cmd.auth_required
 
         # NOTE(dtroyer): Save the auth required state of the _current_ command
@@ -470,10 +471,13 @@ class OpenStackShell(app.App):
         self.client_manager._auth_required = cmd.auth_required
 
         # Validate auth options
-        self.cloud = self.cloud_config.get_one_cloud(
+        self.cloud = self.cloud_config.get_one(
             cloud=self.options.cloud,
             argparse=self.options,
             validate=validate,
+            app_name=self.client_manager._app_name,
+            app_version=self.client_manager._app_version,
+            additional_user_agent=[('osc-lib', version.version_string)],
         )
         # Push the updated args into ClientManager
         self.client_manager._cli_options = self.cloud
diff --git a/osc_lib/tests/command/test_timing.py b/osc_lib/tests/command/test_timing.py
index 0fbf35a..c140f01 100644
--- a/osc_lib/tests/command/test_timing.py
+++ b/osc_lib/tests/command/test_timing.py
@@ -15,12 +15,15 @@
 
 import datetime
 
+from keystoneauth1 import session
+
 from osc_lib.command import timing
 from osc_lib.tests import fakes
 from osc_lib.tests import utils
 
 
-timing_url = 'GET http://localhost:5000'
+timing_method = 'GET'
+timing_url = 'http://localhost:5000'
 timing_elapsed = 0.872809
 
 
@@ -73,9 +76,10 @@ class TestTiming(utils.TestCommand):
         self.assertEqual(datalist, data)
 
     def test_timing_list(self):
-        self.app.timing_data = [(
-            timing_url,
-            datetime.timedelta(microseconds=timing_elapsed * 1000000),
+        self.app.timing_data = [session.RequestTiming(
+            method=timing_method,
+            url=timing_url,
+            elapsed=datetime.timedelta(microseconds=timing_elapsed * 1000000),
         )]
 
         arglist = []
@@ -88,7 +92,7 @@ class TestTiming(utils.TestCommand):
         columns, data = self.cmd.take_action(parsed_args)
         self.assertEqual(self.columns, columns)
         datalist = [
-            (timing_url, timing_elapsed),
+            (timing_method + ' ' + timing_url, timing_elapsed),
             ('Total', timing_elapsed),
         ]
         self.assertEqual(datalist, data)
diff --git a/osc_lib/tests/test_clientmanager.py b/osc_lib/tests/test_clientmanager.py
index e261bdd..6a724f3 100644
--- a/osc_lib/tests/test_clientmanager.py
+++ b/osc_lib/tests/test_clientmanager.py
@@ -24,14 +24,8 @@ from keystoneauth1 import loading
 from keystoneauth1 import noauth
 from keystoneauth1 import token_endpoint
 
-try:
-    from openstack.config import cloud_config
-    from openstack.config import defaults
-    _occ_in_sdk = True
-except ImportError:
-    from os_client_config import cloud_config
-    from os_client_config import defaults
-    _occ_in_sdk = False
+from openstack.config import cloud_config
+from openstack.config import defaults
 from openstack import connection
 
 from osc_lib.api import auth
@@ -423,10 +417,7 @@ class TestClientManagerSDK(utils.TestClientManager):
             auth_required=True,
         )
 
-        if _occ_in_sdk:
-            self.assertIsInstance(
-                client_manager.sdk_connection,
-                connection.Connection,
-            )
-        else:
-            self.assertIsNone(getattr(client_manager, 'sdk_connection', None))
+        self.assertIsInstance(
+            client_manager.sdk_connection,
+            connection.Connection,
+        )
diff --git a/osc_lib/tests/test_shell.py b/osc_lib/tests/test_shell.py
index d519b1b..7f5dce9 100644
--- a/osc_lib/tests/test_shell.py
+++ b/osc_lib/tests/test_shell.py
@@ -22,16 +22,7 @@ import testtools
 from osc_lib import shell
 from osc_lib.tests import utils
 
-# NOTE(dtroyer): Attempt the import to detect if the SDK installed is new
-#                enough to contain the os_client_config code.  If so, use
-#                that path for mocks.
-CONFIG_MOCK_BASE = "openstack.config.loader"
-try:
-    from openstack.config import loader as config   # noqa
-except ImportError:
-    # Fall back to os-client-config
-    CONFIG_MOCK_BASE = "os_client_config.config"
-
+from openstack.config import loader as config   # noqa
 
 DEFAULT_AUTH_URL = "http://127.0.0.1:5000/v2.0/"
 DEFAULT_PROJECT_ID = "xxxx-yyyy-zzzz"
@@ -197,12 +188,12 @@ class TestShellOptions(utils.TestShell):
     def test_empty_auth(self):
         os.environ = {}
         self._assert_initialize_app_arg("", {})
-        self._assert_cloud_config_arg("", {})
+        self._assert_cloud_region_arg("", {})
 
     def test_no_options(self):
         os.environ = {}
         self._assert_initialize_app_arg("", {})
-        self._assert_cloud_config_arg("", {})
+        self._assert_cloud_region_arg("", {})
 
     def test_global_options(self):
         self._test_options_init_app(global_options)
@@ -322,7 +313,7 @@ class TestShellCli(utils.TestShell):
         self.assertEqual('mickey', _shell.options.key)
         self.assertEqual(('mycert', 'mickey'), _shell.client_manager.cert)
 
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_config_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_config_file")
     def test_shell_args_cloud_no_vendor(self, config_mock):
         """Test cloud config options without the vendor file"""
         config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_1))
@@ -371,8 +362,8 @@ class TestShellCli(utils.TestShell):
         self.assertIsNone(_shell.cloud.config['key'])
         self.assertIsNone(_shell.client_manager.cert)
 
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_vendor_file")
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_config_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_vendor_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_config_file")
     def test_shell_args_cloud_public(self, config_mock, public_mock):
         """Test cloud config options with the vendor file"""
         config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2))
@@ -420,8 +411,8 @@ class TestShellCli(utils.TestShell):
         self.assertEqual('mickey', _shell.cloud.config['key'])
         self.assertEqual(('mycert', 'mickey'), _shell.client_manager.cert)
 
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_vendor_file")
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_config_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_vendor_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_config_file")
     def test_shell_args_precedence(self, config_mock, vendor_mock):
         config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2))
         vendor_mock.return_value = ('file.yaml', copy.deepcopy(PUBLIC_1))
@@ -477,8 +468,8 @@ class TestShellCliPrecedence(utils.TestShell):
         }
         self.useFixture(utils.EnvFixture(env.copy()))
 
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_vendor_file")
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_config_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_vendor_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_config_file")
     def test_shell_args_precedence_1(self, config_mock, vendor_mock):
         """Test environment overriding occ"""
         config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2))
@@ -525,8 +516,8 @@ class TestShellCliPrecedence(utils.TestShell):
             _shell.client_manager.region_name,
         )
 
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_vendor_file")
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_config_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_vendor_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_config_file")
     def test_shell_args_precedence_2(self, config_mock, vendor_mock):
         """Test command line overriding environment and occ"""
         config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_2))
@@ -573,8 +564,8 @@ class TestShellCliPrecedence(utils.TestShell):
             _shell.client_manager.region_name,
         )
 
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_vendor_file")
-    @mock.patch(CONFIG_MOCK_BASE + ".OpenStackConfig._load_config_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_vendor_file")
+    @mock.patch("openstack.config.loader.OpenStackConfig._load_config_file")
     def test_shell_args_precedence_3(self, config_mock, vendor_mock):
         """Test command line overriding environment and occ"""
         config_mock.return_value = ('file.yaml', copy.deepcopy(CLOUD_1))
diff --git a/osc_lib/tests/utils/__init__.py b/osc_lib/tests/utils/__init__.py
index 4841f43..5cc6a7c 100644
--- a/osc_lib/tests/utils/__init__.py
+++ b/osc_lib/tests/utils/__init__.py
@@ -24,12 +24,8 @@ from cliff import columns as cliff_columns
 import fixtures
 from keystoneauth1 import loading
 
-try:
-    from openstack.config import cloud_config
-    from openstack.config import defaults
-except ImportError:
-    from os_client_config import cloud_config
-    from os_client_config import defaults
+from openstack.config import cloud_region
+from openstack.config import defaults
 
 from oslo_utils import importutils
 from requests_mock.contrib import fixture
@@ -40,16 +36,6 @@ from osc_lib import clientmanager
 from osc_lib import shell
 from osc_lib.tests import fakes
 
-# NOTE(dtroyer): Attempt the import to detect if the SDK installed is new
-#                enough to contain the os_client_config code.  If so, use
-#                that path for mocks.
-CONFIG_MOCK_BASE = "openstack.config.loader"
-try:
-    from openstack.config import loader as config   # noqa
-except ImportError:
-    # Fall back to os-client-config
-    CONFIG_MOCK_BASE = "os_client_config.config"
-
 
 def fake_execute(shell, cmd):
     """Pretend to execute shell commands."""
@@ -301,9 +287,9 @@ class TestClientManager(TestCase):
         loader = loading.get_plugin_loader(auth_plugin_name)
         auth_plugin = loader.load_from_options(**auth_dict)
         client_manager = self._clientmanager_class()(
-            cli_options=cloud_config.CloudConfig(
+            cli_options=cloud_region.CloudRegion(
                 name='t1',
-                region='1',
+                region_name='1',
                 config=cli_options,
                 auth_plugin=auth_plugin,
             ),
@@ -361,10 +347,10 @@ class TestShell(TestCase):
                     "%s does not match" % k,
                 )
 
-    def _assert_cloud_config_arg(self, cmd_options, default_args):
-        """Check the args passed to cloud_config.get_one_cloud()
+    def _assert_cloud_region_arg(self, cmd_options, default_args):
+        """Check the args passed to OpenStackConfig.get_one()
 
-        The argparse argument to get_one_cloud() is an argparse.Namespace
+        The argparse argument to get_one() is an argparse.Namespace
         object that contains all of the options processed to this point in
         initialize_app().
         """
@@ -373,7 +359,7 @@ class TestShell(TestCase):
         cloud.config = {}
         self.occ_get_one = mock.Mock(return_value=cloud)
         with mock.patch(
-                CONFIG_MOCK_BASE + ".OpenStackConfig.get_one_cloud",
+                "openstack.config.loader.OpenStackConfig.get_one",
                 self.occ_get_one,
         ):
             _shell = make_shell(shell_class=self.shell_class)
@@ -432,7 +418,7 @@ class TestShell(TestCase):
             kwargs = {
                 key: test_opts[opt][0],
             }
-            self._assert_cloud_config_arg(cmd, kwargs)
+            self._assert_cloud_region_arg(cmd, kwargs)
 
     def _test_env_get_one_cloud(self, test_opts):
         """Test environment options sent "to openstack.config"""
@@ -447,4 +433,4 @@ class TestShell(TestCase):
                 opt2env(opt): test_opts[opt][0],
             }
             os.environ = env.copy()
-            self._assert_cloud_config_arg("", kwargs)
+            self._assert_cloud_region_arg("", kwargs)
diff --git a/releasenotes/notes/direct-openstacksdk-535a179f3c645cc0.yaml b/releasenotes/notes/direct-openstacksdk-535a179f3c645cc0.yaml
new file mode 100644
index 0000000..6bfe076
--- /dev/null
+++ b/releasenotes/notes/direct-openstacksdk-535a179f3c645cc0.yaml
@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    The dependency on ``os-client-config`` has been removed in favor of
+    direct use of ``openstacksdk``.
diff --git a/requirements.txt b/requirements.txt
index 7c1351d..387197f 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -6,9 +6,8 @@ six>=1.10.0 # MIT
 
 Babel!=2.4.0,>=2.3.4 # BSD
 cliff!=2.9.0,>=2.8.0 # Apache-2.0
-keystoneauth1>=3.4.0 # Apache-2.0
-openstacksdk>=0.11.2 # Apache-2.0
-os-client-config>=1.28.0 # Apache-2.0
+keystoneauth1>=3.7.0 # Apache-2.0
+openstacksdk>=0.13.0 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 oslo.utils>=3.33.0 # Apache-2.0
 simplejson>=3.5.1 # MIT
diff --git a/tox.ini b/tox.ini
index 9d6263b..f9db43e 100644
--- a/tox.ini
+++ b/tox.ini
@@ -22,7 +22,6 @@ basepython = python3
 commands =
     pip install -q -e "git+file://{toxinidir}/../cliff#egg=cliff"
     pip install -q -e "git+file://{toxinidir}/../keystoneauth#egg=keystoneauth"
-    pip install -q -e "git+file://{toxinidir}/../os-client-config#egg=os_client_config"
     pip install -q -e "git+file://{toxinidir}/../openstacksdk#egg=openstacksdk"
     pip freeze
     ostestr {posargs}