Support hyphens in positional argument names

Currently, when you specify a positional argument with a hyphen in the
name, oslo.config is not able to retrieve a value back for that
argument, even if one is parsed by argparse, because there's a mismatch
between where oslo.config expects to find the value (in the dest, with
underscores, which positional arguments do not have), versus where
argparse puts it in the namespace (using the name, with hyphens).

Change-Id: Ibc44a880ffddfaeffca682ccf3b34525f3f0fe27

2 files changed, 15 insertions, 16 deletions

diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py
index d36245f..06d50bb 100644
--- a/oslo_config/cfg.py
+++ b/oslo_config/cfg.py
@@ -721,6 +721,13 @@ class Opt(object):
         def hyphen(arg):
             return arg if not positional else ''
 
+        # Because we must omit the dest parameter when using a positional
+        # argument, the name supplied for the positional argument must not
+        # include hyphens.
+        if positional:
+            prefix = prefix.replace('-', '_')
+            name = name.replace('-', '_')
+
         args = [hyphen('--') + prefix + name]
         if short:
             args.append(hyphen('-') + short)
diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py
index c9f0414..e304807 100644
--- a/oslo_config/tests/test_cfg.py
+++ b/oslo_config/tests/test_cfg.py
@@ -976,15 +976,11 @@ class PositionalTestCase(BaseTestCase):
 
         self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
         self.assertRaises(SystemExit, self.conf, ['--help'])
-        self.assertIn(' [foo-bar]\n', sys.stdout.getvalue())
+        self.assertIn(' [foo_bar]\n', sys.stdout.getvalue())
 
         self.conf(['baz'])
         self.assertTrue(hasattr(self.conf, 'foo_bar'))
-        # FIXME(dolphm): Due to bug 1676989, this argument cannot be retrieved
-        # by oslo_config.cfg. Instead, the following commented-out code should
-        # work:
-        # self.assertEqual('baz', self.conf.foo_bar)
-        self.assertIsNone(self.conf.foo_bar)
+        self.assertEqual('baz', self.conf.foo_bar)
 
     def test_optional_positional_hyphenated_opt_undefined(self):
         self.conf.register_cli_opt(
@@ -992,7 +988,7 @@ class PositionalTestCase(BaseTestCase):
 
         self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
         self.assertRaises(SystemExit, self.conf, ['--help'])
-        self.assertIn(' [foo-bar]\n', sys.stdout.getvalue())
+        self.assertIn(' [foo_bar]\n', sys.stdout.getvalue())
 
         self.conf([])
         self.assertTrue(hasattr(self.conf, 'foo_bar'))
@@ -1004,15 +1000,11 @@ class PositionalTestCase(BaseTestCase):
 
         self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
         self.assertRaises(SystemExit, self.conf, ['--help'])
-        self.assertIn(' foo-bar\n', sys.stdout.getvalue())
+        self.assertIn(' foo_bar\n', sys.stdout.getvalue())
 
-        # FIXME(dolphm): Due to bug 1676989, this mistakenly raises an
-        # exception, even though the option is clearly defined. Instead, the
-        # following commented out lines should work:
-        # self.conf(['baz'])
-        # self.assertTrue(hasattr(self.conf, 'foo_bar'))
-        # self.assertEqual('baz', self.conf.foo_bar)
-        self.assertRaises(cfg.RequiredOptError, self.conf, ['baz'])
+        self.conf(['baz'])
+        self.assertTrue(hasattr(self.conf, 'foo_bar'))
+        self.assertEqual('baz', self.conf.foo_bar)
 
     def test_required_positional_hyphenated_opt_undefined(self):
         self.conf.register_cli_opt(
@@ -1020,7 +1012,7 @@ class PositionalTestCase(BaseTestCase):
 
         self.useFixture(fixtures.MonkeyPatch('sys.stdout', moves.StringIO()))
         self.assertRaises(SystemExit, self.conf, ['--help'])
-        self.assertIn(' foo-bar\n', sys.stdout.getvalue())
+        self.assertIn(' foo_bar\n', sys.stdout.getvalue())
 
         self.assertRaises(SystemExit, self.conf, [])