Merge lp:~ricardokirkner/configglue/simplified-option-precedence into lp:configglue

Proposed by Ricardo Kirkner
Status: Merged
Approved by: Ricardo Kirkner
Approved revision: 80
Merged at revision: 80
Proposed branch: lp:~ricardokirkner/configglue/simplified-option-precedence
Merge into: lp:configglue
Diff against target: 94 lines (+38/-21)
2 files modified
configglue/glue.py (+10/-21)
configglue/tests/test_schemaconfig.py (+28/-0)
To merge this branch: bzr merge lp:~ricardokirkner/configglue/simplified-option-precedence
Reviewer Review Type Date Requested Status
Ricardo Kirkner Approve
Review via email: mp+69379@code.launchpad.net

Commit message

simplified option precedence

Description of the change

Overview
=========

Simplified option precedence logic and fixed one bug triggered during testing of django_configglue related to it (option not fatal but null misbehaving).

To post a comment you must log in.
Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

Make tarmac happy.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configglue/glue.py'
2--- configglue/glue.py 2011-07-22 15:44:24 +0000
3+++ configglue/glue.py 2011-07-27 00:14:29 +0000
4@@ -23,7 +23,7 @@
5 from optparse import OptionParser
6 from collections import namedtuple
7
8-from configglue.parser import SchemaConfigParser
9+from .parser import SchemaConfigParser
10
11
12 __all__ = [
13@@ -82,23 +82,6 @@
14 value = option.parse(value)
15 parser.set(section.name, option.name, value)
16
17- def set_value_with_precedence(op_value, parser_value, env_value,
18- fatal=True):
19- # 1. op_value != None (only if option is not fatal)
20- # => use op or env value
21- # 2. op_value is None (only if option is fatal)
22- # => use parser or env value
23- if not option.fatal:
24- if op_value != parser_value:
25- # value was overridden via command line
26- set_value(section, option, op_value)
27- elif env_value is not None and env_value != parser_value:
28- # value was overridden via environment variable
29- set_value(section, option, env_value)
30- elif env_value is not None and env_value != parser_value:
31- # value was overridden via environment variable
32- set_value(section, option, env_value)
33-
34 for section in schema.sections():
35 for option in section.options():
36 op_value = getattr(options, opt_name(option))
37@@ -109,9 +92,15 @@
38 env_value = os.environ.get("CONFIGGLUE_{0}".format(
39 long_name(option).upper()))
40
41- assert option.fatal == (op_value is None)
42- set_value_with_precedence(op_value, parser_value, env_value,
43- fatal=option.fatal)
44+ # 1. op value != parser value
45+ # 2. op value == parser value != env value
46+ # 3. op value == parser value == env value or not env value
47+
48+ # if option is fatal, op_value will be None, so skip this case too
49+ if op_value != parser_value and not option.fatal:
50+ set_value(section, option, op_value)
51+ elif env_value is not None and env_value != parser_value:
52+ set_value(section, option, env_value)
53
54 return op, options, args
55
56
57=== modified file 'configglue/tests/test_schemaconfig.py'
58--- configglue/tests/test_schemaconfig.py 2011-07-23 20:26:42 +0000
59+++ configglue/tests/test_schemaconfig.py 2011-07-27 00:14:29 +0000
60@@ -234,6 +234,34 @@
61 finally:
62 sys.argv = _argv
63
64+ def test_glue_environ_precedence_null_option(self):
65+ class MySchema(Schema):
66+ foo = StringOption(null=True)
67+
68+ parser = SchemaConfigParser(MySchema())
69+
70+ with patch.object(os, 'environ', {'CONFIGGLUE_FOO': '42'}):
71+ _argv, sys.argv = sys.argv, ['prognam']
72+ try:
73+ op, options, args = schemaconfigglue(parser)
74+ self.assertEqual(parser.get('__main__', 'foo'), '42')
75+ finally:
76+ sys.argv = _argv
77+
78+ def test_glue_environ_precedence_null_and_fatal_option(self):
79+ class MySchema(Schema):
80+ foo = StringOption(null=True, fatal=True)
81+
82+ parser = SchemaConfigParser(MySchema())
83+
84+ with patch.object(os, 'environ', {'CONFIGGLUE_FOO': '42'}):
85+ _argv, sys.argv = sys.argv, ['prognam']
86+ try:
87+ op, options, args = schemaconfigglue(parser)
88+ self.assertEqual(parser.get('__main__', 'foo'), '42')
89+ finally:
90+ sys.argv = _argv
91+
92 def test_ambiguous_option(self):
93 """Test schemaconfigglue when an ambiguous option is specified."""
94 class MySchema(Schema):

Subscribers

People subscribed via source and target branches