Move try: cfg.get excepts to CephConfg.safe_get

move safe_get from config to conf.CephConfig
update existing uses of safe_get
move try: ... except to cfg.safe_get
remove not needed imports

Signed-off-by: Andrew Woodward (awoodward@mirantis.com)

diff --git a/ceph_deploy/conf.py b/ceph_deploy/conf.py
index 3fcdd2195de85c48a3ee63eab36329b23c52215c..63ea0ba23db5f2f390ae360a023eb187a96a20cd 100644 (file)
--- a/ceph_deploy/conf.py
+++ b/ceph_deploy/conf.py
@@ -19,6 +19,20 @@ class CephConf(ConfigParser.RawConfigParser):
         s = '_'.join(s.split())
         return s
 
+    def safe_get(self, section, key):
+        """
+        Attempt to get a configuration value from a certain section
+        in a ``cfg`` object but returning None if not found. Avoids the need
+        to be doing try/except {ConfigParser Exceptions} every time.
+        """
+        try:
+            #Use full parent function so we can replace it in the class 
+            # if desired
+            return ConfigParser.RawConfigParser.get(self, section, key)
+        except (ConfigParser.NoSectionError,
+                ConfigParser.NoOptionError):
+            return None
+
 
 def parse(fp):
     cfg = CephConf()

diff --git a/ceph_deploy/config.py b/ceph_deploy/config.py
index 7a9e8de34576dfc5a8bd5150b3a134331e6ae1b8..4811f734bb95beb90fc1f69b65167a2f615eb462 100644 (file)
--- a/ceph_deploy/config.py
+++ b/ceph_deploy/config.py
@@ -1,5 +1,4 @@
 import logging
-import ConfigParser
 
 from cStringIO import StringIO
 
@@ -12,19 +11,6 @@ from .sudo_pushy import get_transport
 LOG = logging.getLogger(__name__)
 
 
-def safe_get(cfg, section, key):
-    """
-    Attempt to get a configuration value from a certain section
-    in a ``cfg`` object but returning None if not found. Avoids the need
-    to be doing try/except {ConfigParser Exceptions} every time.
-    """
-    try:
-        return cfg.get(section, key)
-    except (ConfigParser.NoSectionError,
-            ConfigParser.NoOptionError):
-        return None
-
-
 def config_push(args):
     cfg = conf.load(args)
     conf_data = StringIO()

diff --git a/ceph_deploy/mon.py b/ceph_deploy/mon.py
index ca05691d12d7e596d274ce1f041c3734e112e3ea..6eb6de5fbf5b41f4f9063b6862764a402420d488 100644 (file)
--- a/ceph_deploy/mon.py
+++ b/ceph_deploy/mon.py
@@ -1,5 +1,4 @@
 import argparse
-import ConfigParser
 import json
 import logging
 import re
@@ -8,7 +7,7 @@ import socket
 from textwrap import dedent
 import time
 
-from . import conf, config, exc
+from . import conf, exc
 from .cliutil import priority
 from .sudo_pushy import get_transport
 from .util import paths
@@ -52,9 +51,9 @@ def catch_mon_errors(conn, logger, hostname, cfg):
     """
     conn = conn or get_connection(hostname, logger=logger)
     monmap = mon_status_check(conn, logger, hostname).get('monmap', {})
-    mon_initial_members = config.safe_get(cfg, 'global', 'mon_initial_members')
-    public_addr = config.safe_get(cfg, 'global', 'public_addr')
-    public_network = config.safe_get(cfg, 'global', 'public_network')
+    mon_initial_members = cfg.safe_get('global', 'mon_initial_members')
+    public_addr = cfg.safe_get('global', 'public_addr')
+    public_network = cfg.safe_get('global', 'public_network')
     mon_in_monmap = [
         mon.get('name')
         for mon in monmap.get('mons', [{}])
@@ -107,13 +106,8 @@ def mon_create(args):
 
     cfg = conf.load(args)
     if not args.mon:
-        try:
-            mon_initial_members = cfg.get('global', 'mon_initial_members')
-        except (ConfigParser.NoSectionError,
-                ConfigParser.NoOptionError):
-            pass
-        else:
-            args.mon = re.split(r'[,\s]+', mon_initial_members)
+        mon_initial_members = cfg.safe_get('global', 'mon_initial_members')
+        args.mon = re.split(r'[,\s]+', mon_initial_members)
 
     if not args.mon:
         raise exc.NeedHostError()
@@ -287,13 +281,11 @@ def mon_destroy(args):
 
 
 def mon_create_initial(args):
-    try:
-        cfg = conf.load(args)
-        cfg_initial_members = cfg.get('global', 'mon_initial_members')
-        mon_initial_members = re.split(r'[,\s]+', cfg_initial_members)
-    except (ConfigParser.NoSectionError,
-            ConfigParser.NoOptionError):
+    cfg = conf.load(args)
+    cfg_initial_members = cfg.safe_get('global', 'mon_initial_members')
+    if cfg_initial_members is None:
         raise RuntimeError('No `mon initial members` defined in config')
+    mon_initial_members = re.split(r'[,\s]+', cfg_initial_members)
 
     # create them normally through mon_create
     mon_create(args)

diff --git a/ceph_deploy/new.py b/ceph_deploy/new.py
index 9a0945be9d89e64fcfd30c1d8889f187f9e707e7..33d2b6eb88833aec59c6cc615f77f24304e78c25 100644 (file)
--- a/ceph_deploy/new.py
+++ b/ceph_deploy/new.py
@@ -1,4 +1,3 @@
-import ConfigParser
 import errno
 import logging
 import os

diff --git a/ceph_deploy/tests/test_mon.py b/ceph_deploy/tests/test_mon.py
index dde74baec4e7263efb4892ef0088b66af4959446..8a1ab5c28fb2f0143ecbc683edc14a546ca1a112 100644 (file)
--- a/ceph_deploy/tests/test_mon.py
+++ b/ceph_deploy/tests/test_mon.py
@@ -1,7 +1,11 @@
 from ceph_deploy import mon
+from ceph_deploy.conf import CephConf
 from mock import Mock
 
 
+def make_fake_conf():
+    return CephConf()
+
 # NOTE: If at some point we re-use this helper, move it out
 # and make it even more generic
 
@@ -29,18 +33,22 @@ class TestCatchCommonErrors(object):
 
     def test_warn_if_no_intial_members(self):
         fake_conn = make_fake_conn()
-        mon.catch_mon_errors(fake_conn, self.logger, 'host', {})
+        cfg = make_fake_conf()
+        mon.catch_mon_errors(fake_conn, self.logger, 'host', cfg)
         expected_msg = 'is not defined in `mon initial members`'
         self.assert_logger_message(self.logger.warning, expected_msg)
 
     def test_warn_if_not_mon_in_monmap(self):
         fake_conn = make_fake_conn()
-        mon.catch_mon_errors(fake_conn, self.logger, 'host', {})
+        cfg = make_fake_conf()
+        mon.catch_mon_errors(fake_conn, self.logger, 'host', cfg)
         expected_msg = 'does not exist in monmap'
         self.assert_logger_message(self.logger.warning, expected_msg)
 
     def test_warn_if_not_public_addr_and_not_public_netw(self):
         fake_conn = make_fake_conn()
-        mon.catch_mon_errors(fake_conn, self.logger, 'host', {'global': ''})
+        cfg = make_fake_conf()
+        cfg.add_section('global')
+        mon.catch_mon_errors(fake_conn, self.logger, 'host', cfg)
         expected_msg = 'neither `public_addr` nor `public_network`'
         self.assert_logger_message(self.logger.warning, expected_msg)