Repository: tornadoweb/tornado
Pull request #1714 (2016-05-05 11:36:10)

Update options to have same behavior from config file or command line
Currently, Tornado raise an error when we use types option in configuration file such as 
options.define('datetime', type=datetime.datetime)



Patch:
diff --git a/setup.py b/setup.py
index c13eeed0..d2e77f3c 100644
--- a/setup.py
+++ b/setup.py
@@ -155,6 +155,8 @@ setup(
             "gettext_translations/fr_FR/LC_MESSAGES/tornado_test.mo",
             "gettext_translations/fr_FR/LC_MESSAGES/tornado_test.po",
             "options_test.cfg",
+            "options_test_types.cfg",
+            "options_test_types_str.cfg",
             "static/robots.txt",
             "static/sample.xml",
             "static/sample.xml.gz",
diff --git a/tornado/options.py b/tornado/options.py
index 2fbb32ad..759c48d4 100644
--- a/tornado/options.py
+++ b/tornado/options.py
@@ -319,7 +319,17 @@ class OptionParser(object):
         for name in config:
             normalized = self._normalize_name(name)
             if normalized in self._options:
-                self._options[normalized].set(config[name])
+                option = self._options[normalized]
+                if option.multiple:
+                    if not isinstance(config[name], list):
+                        raise Error("Option %r is required to be a list of %s" %
+                                    (option.name, option.type.__name__))
+
+                if type(config[name]) == str and option.type != str:
+                    option.parse(config[name])
+                else:
+                    option.set(config[name])
+
 
         if final:
             self.run_parse_callbacks()
@@ -444,16 +454,19 @@ class _Option(object):
             basestring_type: self._parse_string,
         }.get(self.type, self.type)
         if self.multiple:
-            self._value = []
-            for part in value.split(","):
-                if issubclass(self.type, numbers.Integral):
-                    # allow ranges of the form X:Y (inclusive at both ends)
-                    lo, _, hi = part.partition(":")
-                    lo = _parse(lo)
-                    hi = _parse(hi) if hi else lo
-                    self._value.extend(range(lo, hi + 1))
-                else:
-                    self._value.append(_parse(part))
+            if isinstance(value, list):
+                self._value = value
+            else:
+                self._value = []
+                for part in value.split(","):
+                    if issubclass(self.type, numbers.Integral):
+                        # allow ranges of the form X:Y (inclusive at both ends)
+                        lo, _, hi = part.partition(":")
+                        lo = _parse(lo)
+                        hi = _parse(hi) if hi else lo
+                        self._value.extend(range(lo, hi + 1))
+                    else:
+                        self._value.append(_parse(part))
         else:
             self._value = _parse(value)
         if self.callback is not None:


Test patch:
diff --git a/tornado/test/options_test.py b/tornado/test/options_test.py
index f7b215c5..31799046 100644
--- a/tornado/test/options_test.py
+++ b/tornado/test/options_test.py
@@ -24,6 +24,18 @@ except ImportError:
         mock = None
 
 
+class Email(object):
+   def __init__(self, value):
+        if isinstance(value, str) and '@' in value:
+            self._value = value
+        else:
+            raise ValueError()
+
+   @property
+   def value(self):
+        return self._value
+
+
 class OptionsTest(unittest.TestCase):
     def test_parse_command_line(self):
         options = OptionParser()
@@ -189,7 +201,7 @@ class OptionsTest(unittest.TestCase):
             self.assertEqual(options.foo, 5)
         self.assertEqual(options.foo, 2)
 
-    def test_types(self):
+    def _define_options(self):
         options = OptionParser()
         options.define('str', type=str)
         options.define('basestring', type=basestring_type)
@@ -197,13 +209,11 @@ class OptionsTest(unittest.TestCase):
         options.define('float', type=float)
         options.define('datetime', type=datetime.datetime)
         options.define('timedelta', type=datetime.timedelta)
-        options.parse_command_line(['main.py',
-                                    '--str=asdf',
-                                    '--basestring=qwer',
-                                    '--int=42',
-                                    '--float=1.5',
-                                    '--datetime=2013-04-28 05:16',
-                                    '--timedelta=45s'])
+        options.define('email', type=Email)
+        options.define('list-of-int', type=int, multiple=True)
+        return options
+
+    def _check_options_values(self, options):
         self.assertEqual(options.str, 'asdf')
         self.assertEqual(options.basestring, 'qwer')
         self.assertEqual(options.int, 42)
@@ -211,6 +221,30 @@ class OptionsTest(unittest.TestCase):
         self.assertEqual(options.datetime,
                          datetime.datetime(2013, 4, 28, 5, 16))
         self.assertEqual(options.timedelta, datetime.timedelta(seconds=45))
+        self.assertEqual(options.email.value, 'tornado@web.com')
+        self.assertTrue(isinstance(options.email, Email))
+        self.assertEqual(options.list_of_int, [1, 2, 3])
+
+    def test_types(self):
+        options = self._define_options()
+        options.parse_command_line(['main.py',
+                                    '--str=asdf',
+                                    '--basestring=qwer',
+                                    '--int=42',
+                                    '--float=1.5',
+                                    '--datetime=2013-04-28 05:16',
+                                    '--timedelta=45s',
+                                    '--email=tornado@web.com',
+                                    '--list-of-int=1,2,3'])
+        self._check_options_values(options)
+
+    def test_types_with_conf_file(self):
+        for config_file_name in ("options_test_types.cfg",
+                                 "options_test_types_str.cfg"):
+            options = self._define_options()
+            options.parse_config_file(os.path.join(os.path.dirname(__file__),
+                                      config_file_name))
+            self._check_options_values(options)
 
     def test_multiple_string(self):
         options = OptionParser()
diff --git a/tornado/test/options_test_types.cfg b/tornado/test/options_test_types.cfg
new file mode 100644
index 00000000..e1d53cb9
--- /dev/null
+++ b/tornado/test/options_test_types.cfg
@@ -0,0 +1,11 @@
+from datetime import datetime, timedelta
+from tornado.test.options_test import Email
+
+str = 'asdf'
+basestring = 'qwer'
+int = 42
+float = 1.5
+datetime = datetime(2013, 4, 28, 5, 16)
+timedelta = timedelta(0, 45)
+email = Email('tornado@web.com')
+list_of_int = [1, 2, 3]
diff --git a/tornado/test/options_test_types_str.cfg b/tornado/test/options_test_types_str.cfg
new file mode 100644
index 00000000..f02873a4
--- /dev/null
+++ b/tornado/test/options_test_types_str.cfg
@@ -0,0 +1,8 @@
+str = 'asdf'
+basestring = 'qwer'
+int = 42
+float = 1.5
+datetime = '2013-04-28 05:16'
+timedelta = '45s'
+email = 'tornado@web.com'
+list_of_int = [1, 2, 3]