configglue team mailing list archive
-
configglue team
-
Mailing list archive
-
Message #00162
[Merge] lp:~ricardokirkner/configglue/757955-fatal-help into lp:configglue
Ricardo Kirkner has proposed merging lp:~ricardokirkner/configglue/757955-fatal-help into lp:configglue.
Requested reviews:
Configglue developers (configglue)
Related bugs:
Bug #757955 in configglue: "fatal kwarg on option does not allow --help on cli for non existing config files"
https://bugs.launchpad.net/configglue/+bug/757955
For more details, see:
https://code.launchpad.net/~ricardokirkner/configglue/757955-fatal-help/+merge/65137
This branch fixes bug #757955.
It introduces better handling of fatal options, so commandline integration will not fail until it must, and also better error reporting so that more information is presented to the user about the error.
--
https://code.launchpad.net/~ricardokirkner/configglue/757955-fatal-help/+merge/65137
Your team Configglue developers is requested to review the proposed merge of lp:~ricardokirkner/configglue/757955-fatal-help into lp:configglue.
=== modified file 'configglue/app/tests/test_base.py'
--- configglue/app/tests/test_base.py 2011-06-18 15:48:15 +0000
+++ configglue/app/tests/test_base.py 2011-06-19 21:28:32 +0000
@@ -1,5 +1,5 @@
import user
-from ConfigParser import NoSectionError
+from ConfigParser import NoOptionError
from unittest import TestCase
from mock import (
@@ -64,7 +64,7 @@
def test_glue_invalid_config(self):
class MySchema(Schema):
foo = IntOption(fatal=True)
- self.assertRaises(NoSectionError, make_app, schema=MySchema)
+ self.assertRaises(SystemExit, make_app, schema=MySchema)
def test_get_config_files(self):
app = make_app()
=== modified file 'configglue/pyschema/glue.py'
--- configglue/pyschema/glue.py 2011-06-18 20:20:34 +0000
+++ configglue/pyschema/glue.py 2011-06-19 21:28:32 +0000
@@ -17,6 +17,10 @@
import os
import sys
+from ConfigParser import (
+ NoOptionError,
+ NoSectionError,
+)
from optparse import OptionParser
from collections import namedtuple
@@ -63,7 +67,8 @@
kwargs = {}
if option.help:
kwargs['help'] = option.help
- kwargs['default'] = parser.get(section.name, option.name)
+ if not option.fatal:
+ kwargs['default'] = parser.get(section.name, option.name)
kwargs['action'] = option.action
og.add_option('--' + long_name(option), **kwargs)
options, args = op.parse_args(argv)
@@ -81,7 +86,10 @@
# 3. op value == parser value == env value or not env value
op_value = getattr(options, opt_name(option))
- parser_value = parser.get(section.name, option.name)
+ try:
+ parser_value = parser.get(section.name, option.name)
+ except (NoSectionError, NoOptionError):
+ parser_value = None
env_value = os.environ.get("CONFIGGLUE_{0}".format(
long_name(option).upper()))
@@ -111,5 +119,5 @@
parser, opts, args = schemaconfigglue(scp, op=op)
is_valid, reasons = scp.is_valid(report=True)
if not is_valid:
- parser.error(reasons[0])
+ parser.error('\n'.join(reasons))
return SchemaGlue(scp, parser, opts, args)
=== modified file 'configglue/pyschema/parser.py'
--- configglue/pyschema/parser.py 2011-06-19 16:38:52 +0000
+++ configglue/pyschema/parser.py 2011-06-19 21:28:32 +0000
@@ -89,13 +89,19 @@
schema_sections.update(skip_sections)
sections_match = (parsed_sections == schema_sections)
if not sections_match:
- error_msg = "Sections in configuration do not match schema: %s"
unmatched_sections = list(parsed_sections - schema_sections)
- error_value = ', '.join(unmatched_sections)
- errors.append(error_msg % error_value)
+ if len(unmatched_sections) > 0:
+ error_msg = "Sections in configuration are missing from schema: %s"
+ error_value = ', '.join(unmatched_sections)
+ errors.append(error_msg % error_value)
+ unmatched_sections = list(schema_sections - parsed_sections)
+ if len(unmatched_sections) > 0:
+ error_msg = "Sections in schema are missing from configuration: %s"
+ error_value = ', '.join(unmatched_sections)
+ errors.append(error_msg % error_value)
valid &= sections_match
- for name in parsed_sections:
+ for name in list(parsed_sections.union(schema_sections)):
if name not in skip_sections:
if not self.schema.has_section(name):
# this should have been reported before
@@ -103,7 +109,10 @@
continue
section = self.schema.section(name)
- parsed_options = set(self.options(name))
+ try:
+ parsed_options = set(self.options(name))
+ except:
+ parsed_options = set([])
schema_options = set(section.options())
fatal_options = set(opt.name for opt in schema_options
@@ -139,9 +148,8 @@
self.parse_all()
except Exception, e:
- if valid:
- errors.append(e)
- valid = False
+ errors.append(str(e))
+ valid = False
if report:
return valid, errors
@@ -446,35 +454,19 @@
return interpolated
def _get_default(self, section, option):
- # mark the value as not initialized to be able to have a None default
- marker = object()
- value = marker
-
# cater for 'special' sections
- if section == '__main__':
- opt = getattr(self.schema, option, None)
- if opt is not None and not opt.fatal:
- value = opt.default
- elif section == '__noschema__':
+ if section == '__noschema__':
value = super(SchemaConfigParser, self).get(section, option)
- else:
- try:
- opt = self.schema.section(section).option(option)
- if not opt.fatal:
- value = opt.default
- except Exception:
- pass
-
- if value is marker:
- # value was not set, so either section or option was not found
- # or option was required (fatal set to True)
- #if self.schema.has_section(section):
- # raise NoOptionError(option, section)
- #else:
- # raise NoSectionError(section)
- return None
- else:
- return value
+ return value
+
+ # any other section
+ opt = self.schema.section(section).option(option)
+ if not opt.fatal:
+ value = opt.default
+ return value
+
+ # no default value found, raise an error
+ raise NoOptionError(option, section)
def get(self, section, option, raw=False, vars=None, parse=True):
"""Return the parsed value of an option.
@@ -507,15 +499,9 @@
# option not found in config, try to get its default value from
# schema
value = self._get_default(section, option)
- if value is None:
- raise
# value found, so section and option exist
# add it to the config
- if not self.has_section(section):
- # Don't call .add_section here because 2.6 complains
- # about sections called '__main__'
- self._sections[section] = {}
self.set(section, option, value)
# interpolate environment variables
@@ -545,6 +531,10 @@
# cast value to a string because SafeConfigParser only allows
# strings to be set
str_value = option_obj.to_string(value)
+ if not self.has_section(section):
+ # Don't call .add_section here because 2.6 complains
+ # about sections called '__main__'
+ self._sections[section] = {}
super(SchemaConfigParser, self).set(section, option, str_value)
filename = self.locate(option)
self._dirty[filename][section][option] = str_value
=== modified file 'configglue/pyschema/schema.py'
--- configglue/pyschema/schema.py 2011-06-19 16:46:01 +0000
+++ configglue/pyschema/schema.py 2011-06-19 21:28:32 +0000
@@ -15,6 +15,10 @@
#
###############################################################################
+from ConfigParser import (
+ NoSectionError,
+ NoOptionError,
+)
from copy import deepcopy
from inspect import getmembers
from warnings import warn
@@ -175,7 +179,8 @@
def section(self, name):
"""Return a Section by name"""
section = self._sections.get(name)
- assert section is not None, "Invalid Section name '%s'" % name
+ if section is None:
+ raise NoSectionError(name)
return section
def sections(self):
@@ -242,7 +247,8 @@
def option(self, name):
"""Return a Option by name"""
opt = getattr(self, name, None)
- assert opt is not None, "Invalid Option name '%s'" % name
+ if opt is None:
+ raise NoOptionError(name, self.name)
return opt
def options(self):
=== modified file 'configglue/tests/pyschema/test_parser.py'
--- configglue/tests/pyschema/test_parser.py 2011-06-19 17:38:47 +0000
+++ configglue/tests/pyschema/test_parser.py 2011-06-19 21:28:32 +0000
@@ -705,14 +705,12 @@
self.assertEqual(default, expected)
def test_get_default_no_option(self):
- expected = None
- default = self.parser._get_default('__main__', 'bar')
- self.assertEqual(default, expected)
+ self.assertRaises(NoOptionError, self.parser._get_default,
+ '__main__', 'bar')
def test_get_default_no_section(self):
- expected = None
- default = self.parser._get_default('foo', 'bar')
- self.assertEqual(default, expected)
+ self.assertRaises(NoSectionError, self.parser._get_default,
+ 'foo', 'bar')
def test_multi_file_dict_config(self):
"""Test parsing a dict option spanning multiple files."""
@@ -1023,8 +1021,8 @@
valid, errors = self.parser.is_valid(report=True)
self.assertFalse(valid)
- self.assertEqual(errors,
- [u'Sections in configuration do not match schema: bar'])
+ self.assertEqual(errors[0],
+ u'Sections in configuration are missing from schema: bar')
def test_different_sections(self):
config = StringIO("[__main__]\nfoo=1\n[bar]\nbaz=2")
=== modified file 'configglue/tests/pyschema/test_schema.py'
--- configglue/tests/pyschema/test_schema.py 2011-06-19 16:46:01 +0000
+++ configglue/tests/pyschema/test_schema.py 2011-06-19 21:28:32 +0000
@@ -17,6 +17,7 @@
###############################################################################
import unittest
+from ConfigParser import NoOptionError
from StringIO import StringIO
from configglue.pyschema.parser import (
@@ -962,7 +963,7 @@
section.foo = IntOption()
self.assertEqual(section.option('foo'), section.foo)
- self.assertRaises(AssertionError, section.option, 'bar')
+ self.assertRaises(NoOptionError, section.option, 'bar')
def test_options(self):
"""Test Section options method."""
=== modified file 'configglue/tests/pyschema/test_schemaconfig.py'
--- configglue/tests/pyschema/test_schemaconfig.py 2011-06-19 17:38:47 +0000
+++ configglue/tests/pyschema/test_schemaconfig.py 2011-06-19 21:28:32 +0000
@@ -263,6 +263,28 @@
output = stdout.read()
self.assertTrue(output.startswith('Usage:'))
+ def test_help_with_fatal(self):
+ """Test schemaconfigglue with --help and an undefined fatal option."""
+ class MySchema(Schema):
+ foo = IntOption(fatal=True)
+
+ self.parser = SchemaConfigParser(MySchema())
+
+ # replace stdout to capture its value
+ stdout = StringIO()
+ _stdout = sys.stdout
+ sys.stdout = stdout
+ # call the method and assert its value
+ self.assertRaises(SystemExit, schemaconfigglue, self.parser,
+ argv=['--help'])
+ # replace stdout again to cleanup
+ sys.stdout = _stdout
+
+ # assert the value of stdout is correct
+ stdout.seek(0)
+ output = stdout.read()
+ self.assertTrue(output.startswith('Usage:'))
+
def test_parser_set_with_encoding(self):
"""Test schemaconfigglue override an option with a non-ascii value."""
class MySchema(Schema):
Follow ups
