From a84b5d73f18780e96f6e942e69f74321c8fb5f1f Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Thu, 30 Mar 2017 20:17:16 +0000 Subject: [PATCH] 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 --- oslo_config/cfg.py | 7 +++++++ oslo_config/tests/test_cfg.py | 24 ++++++++---------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py index d36245f9..06d50bb8 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 c9f04146..e304807f 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, [])