configglue team mailing list archive

[Ricardo Kirkner <ricardo.kirkner@xxxxxxxxxxxxx>]

Ricardo Kirkner has proposed merging lp:~ricardokirkner/configglue/simplified-option-precedence into lp:configglue.

Requested reviews:
  Configglue developers (configglue)

For more details, see:
https://code.launchpad.net/~ricardokirkner/configglue/simplified-option-precedence/+merge/69379

Overview
=========

Simplified option precedence logic and fixed one bug triggered during testing of django_configglue related to it (option not fatal but null misbehaving).
-- 
https://code.launchpad.net/~ricardokirkner/configglue/simplified-option-precedence/+merge/69379
Your team Configglue developers is requested to review the proposed merge of lp:~ricardokirkner/configglue/simplified-option-precedence into lp:configglue.

=== modified file 'configglue/glue.py'
--- configglue/glue.py	2011-07-22 15:44:24 +0000
+++ configglue/glue.py	2011-07-27 00:14:29 +0000
@@ -23,7 +23,7 @@
 from optparse import OptionParser
 from collections import namedtuple
 
-from configglue.parser import SchemaConfigParser
+from .parser import SchemaConfigParser
 
 
 __all__ = [
@@ -82,23 +82,6 @@
             value = option.parse(value)
         parser.set(section.name, option.name, value)
 
-    def set_value_with_precedence(op_value, parser_value, env_value,
-            fatal=True):
-        # 1. op_value != None (only if option is not fatal)
-        # => use op or env value
-        # 2. op_value is None (only if option is fatal)
-        # => use parser or env value
-        if not option.fatal:
-            if op_value != parser_value:
-                # value was overridden via command line
-                set_value(section, option, op_value)
-            elif env_value is not None and env_value != parser_value:
-                # value was overridden via environment variable
-                set_value(section, option, env_value)
-        elif env_value is not None and env_value != parser_value:
-            # value was overridden via environment variable
-            set_value(section, option, env_value)
-
     for section in schema.sections():
         for option in section.options():
             op_value = getattr(options, opt_name(option))
@@ -109,9 +92,15 @@
             env_value = os.environ.get("CONFIGGLUE_{0}".format(
                 long_name(option).upper()))
 
-            assert option.fatal == (op_value is None)
-            set_value_with_precedence(op_value, parser_value, env_value,
-                fatal=option.fatal)
+            # 1. op value != parser value
+            # 2. op value == parser value != env value
+            # 3. op value == parser value == env value or not env value
+
+            # if option is fatal, op_value will be None, so skip this case too
+            if op_value != parser_value and not option.fatal:
+                set_value(section, option, op_value)
+            elif env_value is not None and env_value != parser_value:
+                set_value(section, option, env_value)
 
     return op, options, args
 

=== modified file 'configglue/tests/test_schemaconfig.py'
--- configglue/tests/test_schemaconfig.py	2011-07-23 20:26:42 +0000
+++ configglue/tests/test_schemaconfig.py	2011-07-27 00:14:29 +0000
@@ -234,6 +234,34 @@
             finally:
                 sys.argv = _argv
 
+    def test_glue_environ_precedence_null_option(self):
+        class MySchema(Schema):
+            foo = StringOption(null=True)
+
+        parser = SchemaConfigParser(MySchema())
+
+        with patch.object(os, 'environ', {'CONFIGGLUE_FOO': '42'}):
+            _argv, sys.argv = sys.argv, ['prognam']
+            try:
+                op, options, args = schemaconfigglue(parser)
+                self.assertEqual(parser.get('__main__', 'foo'), '42')
+            finally:
+                sys.argv = _argv
+
+    def test_glue_environ_precedence_null_and_fatal_option(self):
+        class MySchema(Schema):
+            foo = StringOption(null=True, fatal=True)
+
+        parser = SchemaConfigParser(MySchema())
+
+        with patch.object(os, 'environ', {'CONFIGGLUE_FOO': '42'}):
+            _argv, sys.argv = sys.argv, ['prognam']
+            try:
+                op, options, args = schemaconfigglue(parser)
+                self.assertEqual(parser.get('__main__', 'foo'), '42')
+            finally:
+                sys.argv = _argv
+
     def test_ambiguous_option(self):
         """Test schemaconfigglue when an ambiguous option is specified."""
         class MySchema(Schema):