Merge "Parse "dispatcher" options correctly from InfluxDB client"

5 files changed, 89 insertions, 39 deletions

diff --git a/api/utils/influx.py b/api/utils/influx.py
index 9bc6e9abe..f391ad972 100644
--- a/api/utils/influx.py
+++ b/api/utils/influx.py
@@ -6,15 +6,19 @@
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
 ##############################################################################
-from __future__ import absolute_import
 
 import logging
 
-import six.moves.configparser as ConfigParser
-from six.moves.urllib.parse import urlsplit
-from influxdb import InfluxDBClient
+from six.moves import configparser as ConfigParser
+# NOTE(ralonsoh): pylint E0401 import error
+# https://github.com/PyCQA/pylint/issues/1640
+from six.moves.urllib.parse import urlsplit  # pylint: disable=relative-import
+from influxdb import client as influxdb_client
 
 from yardstick.common import constants as consts
+from yardstick.common import exceptions
+from yardstick import dispatcher
+
 
 logger = logging.getLogger(__name__)
 
@@ -23,22 +27,22 @@ def get_data_db_client():
     parser = ConfigParser.ConfigParser()
     try:
         parser.read(consts.CONF_FILE)
-
-        if parser.get('DEFAULT', 'dispatcher') != 'influxdb':
-            raise RuntimeError
-
-        return _get_client(parser)
+        return _get_influxdb_client(parser)
     except ConfigParser.NoOptionError:
-        logger.error('can not find the key')
+        logger.error('Can not find the key')
         raise
 
 
-def _get_client(parser):
+def _get_influxdb_client(parser):
+    if dispatcher.INFLUXDB not in parser.get('DEFAULT', 'dispatcher'):
+        raise exceptions.InfluxDBConfigurationMissing()
+
     ip = _get_ip(parser.get('dispatcher_influxdb', 'target'))
     user = parser.get('dispatcher_influxdb', 'username')
     password = parser.get('dispatcher_influxdb', 'password')
     db_name = parser.get('dispatcher_influxdb', 'db_name')
-    return InfluxDBClient(ip, consts.INFLUXDB_PORT, user, password, db_name)
+    return influxdb_client.InfluxDBClient(ip, consts.INFLUXDB_PORT, user,
+                                          password, db_name)
 
 
 def _get_ip(url):
diff --git a/yardstick/common/exceptions.py b/yardstick/common/exceptions.py
index 36c7eef05..c7ba56268 100644
--- a/yardstick/common/exceptions.py
+++ b/yardstick/common/exceptions.py
@@ -63,6 +63,11 @@ class FunctionNotImplemented(YardstickException):
                '"%(class_name)" class.')
 
 
+class InfluxDBConfigurationMissing(YardstickException):
+    message = ('InfluxDB configuration is not available. Add "influxdb" as '
+               'a dispatcher and the configuration section')
+
+
 class YardstickBannedModuleImported(YardstickException):
     # pragma: no cover
     message = 'Module "%(module)s" cannnot be imported. Reason: "%(reason)s"'
diff --git a/yardstick/dispatcher/__init__.py b/yardstick/dispatcher/__init__.py
index dfb130760..837a4397c 100644
--- a/yardstick/dispatcher/__init__.py
+++ b/yardstick/dispatcher/__init__.py
@@ -7,12 +7,12 @@
 # http://www.apache.org/licenses/LICENSE-2.0
 ##############################################################################
 
-from __future__ import absolute_import
 from oslo_config import cfg
 
 import yardstick.common.utils as utils
 
-utils.import_modules_from_package("yardstick.dispatcher")
+utils.import_modules_from_package('yardstick.dispatcher')
+
 
 CONF = cfg.CONF
 OPTS = [
@@ -21,3 +21,8 @@ OPTS = [
                help='Dispatcher to store data.'),
 ]
 CONF.register_opts(OPTS)
+
+# Dispatchers
+FILE = 'file'
+HTTP = 'http'
+INFLUXDB = 'influxdb'
diff --git a/yardstick/tests/unit/apiserver/utils/__init__.py b/yardstick/tests/unit/apiserver/utils/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/yardstick/tests/unit/apiserver/utils/__init__.py
diff --git a/yardstick/tests/unit/apiserver/utils/test_influx.py b/yardstick/tests/unit/apiserver/utils/test_influx.py
index dce6c1cec..95105d8ae 100644
--- a/yardstick/tests/unit/apiserver/utils/test_influx.py
+++ b/yardstick/tests/unit/apiserver/utils/test_influx.py
@@ -6,28 +6,48 @@
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
 ##############################################################################
-import unittest
+
+from influxdb import client as influxdb_client
 import mock
+from six.moves import configparser
 
 from api.utils import influx
-from six.moves import configparser as ConfigParser
+from yardstick.common import constants
+from yardstick.common import exceptions
+from yardstick import dispatcher
+from yardstick.tests.unit import base
+
 
+class GetDataDbClientTestCase(base.BaseUnitTestCase):
 
-class GetDataDbClientTestCase(unittest.TestCase):
+    @mock.patch.object(influx, '_get_influxdb_client',
+                       return_value='fake_client')
+    @mock.patch.object(influx.ConfigParser, 'ConfigParser')
+    def test_get_data_db_client(self, mock_parser, mock_get_client):
+        _mock_parser = mock.Mock()
+        mock_parser.return_value = _mock_parser
 
-    @mock.patch('api.utils.influx.ConfigParser')
-    def test_get_data_db_client_dispatcher_not_influxdb(self, mock_parser):
-        mock_parser.ConfigParser().get.return_value = 'file'
-        # reset exception to avoid
-        # TypeError: catching classes that do not inherit from BaseException
-        mock_parser.NoOptionError = ConfigParser.NoOptionError
-        try:
+        self.assertEqual('fake_client', influx.get_data_db_client())
+        _mock_parser.read.assert_called_once_with(constants.CONF_FILE)
+        mock_get_client.assert_called_once_with(_mock_parser)
+
+    @mock.patch.object(influx, '_get_influxdb_client',
+                       return_value='fake_client')
+    @mock.patch.object(influx.ConfigParser, 'ConfigParser')
+    def test_get_data_db_client_parsing_error(self, mock_parser,
+                                              mock_get_client):
+        _mock_parser = mock.Mock()
+        mock_parser.return_value = _mock_parser
+        mock_parser.NoOptionError = configparser.NoOptionError
+        mock_get_client.side_effect = configparser.NoOptionError('option', 'section')
+        with self.assertRaises(configparser.NoOptionError):
             influx.get_data_db_client()
-        except Exception as e:  # pylint: disable=broad-except
-            self.assertIsInstance(e, RuntimeError)
+
+        _mock_parser.read.assert_called_once_with(constants.CONF_FILE)
+        mock_get_client.assert_called_once_with(_mock_parser)
 
 
-class GetIpTestCase(unittest.TestCase):
+class GetIpTestCase(base.BaseUnitTestCase):
 
     def test_get_url(self):
         url = 'http://localhost:8086/hello'
@@ -37,16 +57,32 @@ class GetIpTestCase(unittest.TestCase):
         self.assertEqual(result, output)
 
 
-class QueryTestCase(unittest.TestCase):
+class GetInfluxdbTestCase(base.BaseUnitTestCase):
+
+    @mock.patch.object(influxdb_client, 'InfluxDBClient',
+                       return_value='idb_client')
+    @mock.patch.object(influx, '_get_ip', return_value='fake_ip')
+    def test_get_influxdb_client(self, mock_get_ip, mock_client):
+        mock_parser = mock.Mock()
+        mock_parser.get.side_effect = [dispatcher.INFLUXDB, 'target', 'user',
+                                       'pass', 'db_name']
+
+        self.assertEqual('idb_client',
+                         influx._get_influxdb_client(mock_parser))
+        mock_client.assert_called_once_with('fake_ip', constants.INFLUXDB_PORT,
+                                            'user', 'pass', 'db_name')
+        mock_get_ip.assert_called_once_with('target')
+        mock_parser.get.assert_has_calls([
+            mock.call('DEFAULT', 'dispatcher'),
+            mock.call('dispatcher_influxdb', 'target'),
+            mock.call('dispatcher_influxdb', 'username'),
+            mock.call('dispatcher_influxdb', 'password'),
+            mock.call('dispatcher_influxdb', 'db_name')])
+
+    def test_get_influxdb_client_no_influxdb_client(self):
+        mock_parser = mock.Mock()
+        mock_parser.get.return_value = dispatcher.FILE
 
-    @mock.patch('api.utils.influx.ConfigParser')
-    def test_query_dispatcher_not_influxdb(self, mock_parser):
-        mock_parser.ConfigParser().get.return_value = 'file'
-        # reset exception to avoid
-        # TypeError: catching classes that do not inherit from BaseException
-        mock_parser.NoOptionError = ConfigParser.NoOptionError
-        try:
-            sql = 'select * form tasklist'
-            influx.query(sql)
-        except Exception as e:  # pylint: disable=broad-except
-            self.assertIsInstance(e, RuntimeError)
+        with self.assertRaises(exceptions.InfluxDBConfigurationMissing):
+            influx._get_influxdb_client(mock_parser)
+        mock_parser.get.assert_called_once_with('DEFAULT', 'dispatcher')