Fixed bug that caused file-based configuration options to be wrongly …

…overriden by defaulted program args.
Jahaja · Oct 23, 2014 · f20fa76 · f20fa76
1 parent e13ce02
commit f20fa76
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 16 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,7 @@
+--- v0.6.1 [Not released yet] ---
+
+* [FIX] Fixed bug that caused file-based configuration options to be wrongly overriden by defaulted program args.
+
 --- v0.6.0 [2014-09-24] ---
 
 * [NEW] Major updates to the UI design.

diff --git a/psdash/run.py b/psdash/run.py
@@ -24,6 +24,8 @@ class PsDashRunner(object):
     DEFAULT_LOG_INTERVAL = 60
     DEFAULT_NET_IO_COUNTER_INTERVAL = 3
     DEFAULT_REGISTER_INTERVAL = 60
+    DEFAULT_BIND_HOST = '0.0.0.0'
+    DEFAULT_PORT = 5000
     LOCAL_NODE = 'localhost'
 
     @classmethod
@@ -49,7 +51,7 @@ def _get_args(cls, args):
             '-l', '--log',
             action='append',
             dest='logs',
-            default=[],
+            default=None,
             metavar='path',
             help='log files to make available for psdash. Patterns (e.g. /var/log/**/*.log) are supported. '
                  'This option can be used multiple times.'
@@ -58,7 +60,7 @@ def _get_args(cls, args):
             '-b', '--bind',
             action='store',
             dest='bind_host',
-            default='0.0.0.0',
+            default=None,
             metavar='host',
             help='host to bind to. Defaults to 0.0.0.0 (all interfaces).'
         )
@@ -67,7 +69,7 @@ def _get_args(cls, args):
             action='store',
             type=int,
             dest='port',
-            default=5000,
+            default=None,
             metavar='port',
             help='port to listen on. Defaults to 5000.'
         )
@@ -105,8 +107,9 @@ def _get_args(cls, args):
     def _load_args_config(self, args):
         config = {}
         for k, v in vars(self._get_args(args)).iteritems():
-            key = 'PSDASH_%s' % k.upper() if k != 'debug' else 'DEBUG'
-            config[key] = v
+            if v:
+                key = 'PSDASH_%s' % k.upper() if k != 'debug' else 'DEBUG'
+                config[key] = v
         return config
 
     def _setup_nodes(self):
@@ -188,10 +191,11 @@ def _setup_workers(self):
         net_io_interval = self.app.config.get('PSDASH_NET_IO_COUNTER_INTERVAL', self.DEFAULT_NET_IO_COUNTER_INTERVAL)
         gevent.spawn_later(net_io_interval, self._net_io_counters_worker, net_io_interval)
 
-        logs_interval = self.app.config.get('PSDASH_LOGS_INTERVAL', self.DEFAULT_LOG_INTERVAL)
-        gevent.spawn_later(logs_interval, self._logs_worker, logs_interval)
+        if 'PSDASH_LOGS' in self.app.config:
+            logs_interval = self.app.config.get('PSDASH_LOGS_INTERVAL', self.DEFAULT_LOG_INTERVAL)
+            gevent.spawn_later(logs_interval, self._logs_worker, logs_interval)
 
-        if self.app.config['PSDASH_AGENT']:
+        if self.app.config.get('PSDASH_AGENT'):
             register_interval = self.app.config.get('PSDASH_REGISTER_INTERVAL', self.DEFAULT_REGISTER_INTERVAL)
             gevent.spawn_later(register_interval, self._register_agent_worker, register_interval)
 
@@ -223,13 +227,13 @@ def _net_io_counters_worker(self, sleep_interval):
             gevent.sleep(sleep_interval)
 
     def _register_agent(self):
-        register_name = self.app.config['PSDASH_REGISTER_AS']
+        register_name = self.app.config.get('PSDASH_REGISTER_AS')
         if not register_name:
             register_name = socket.gethostname()
 
         url_args = {
             'name': register_name,
-            'port': self.app.config['PSDASH_PORT'],
+            'port': self.app.config.get('PSDASH_PORT', self.DEFAULT_PORT),
         }
         register_url = '%s/register?%s' % (self.app.config['PSDASH_REGISTER_TO'], urllib.urlencode(url_args))
 
@@ -252,12 +256,13 @@ def _register_agent(self):
     def _run_rpc(self):
         logger.info("Starting RPC server (agent mode)")
 
-        if self.app.config['PSDASH_REGISTER_TO']:
+        if 'PSDASH_REGISTER_TO' in self.app.config:
             self._register_agent()
 
         service = self.get_local_node().get_service()
         self.server = zerorpc.Server(service)
-        self.server.bind('tcp://%s:%s' % (self.app.config['PSDASH_BIND_HOST'], self.app.config['PSDASH_PORT']))
+        self.server.bind('tcp://%s:%s' % (self.app.config.get('PSDASH_BIND_HOST', self.DEFAULT_BIND_HOST),
+                                          self.app.config.get('PSDASH_PORT', self.DEFAULT_PORT)))
         self.server.run()
 
     def _run_web(self):
@@ -271,8 +276,12 @@ def _run_web(self):
                 'certfile': self.app.config.get('PSDASH_HTTPS_CERTFILE')
             }
 
+        listen_to = (
+            self.app.config.get('PSDASH_BIND_HOST', self.DEFAULT_BIND_HOST),
+            self.app.config.get('PSDASH_PORT', self.DEFAULT_PORT)
+        )
         self.server = WSGIServer(
-            (self.app.config['PSDASH_BIND_HOST'], self.app.config['PSDASH_PORT']),
+            listen_to,
             application=self.app,
             log=log,
             **ssl_args
@@ -286,10 +295,10 @@ def run(self):
         self._setup_workers()
 
         logger.info('Listening on %s:%s',
-                    self.app.config['PSDASH_BIND_HOST'],
-                    self.app.config['PSDASH_PORT'])
+                    self.app.config.get('PSDASH_BIND_HOST', self.DEFAULT_BIND_HOST),
+                    self.app.config.get('PSDASH_PORT', self.DEFAULT_PORT))
 
-        if self.app.config['PSDASH_AGENT']:
+        if self.app.config.get('PSDASH_AGENT'):
             return self._run_rpc()
         else:
             return self._run_web()

diff --git a/tests/test_run.py b/tests/test_run.py
@@ -1,3 +1,4 @@
+import os
 from psdash.run import PsDashRunner
 from psdash.node import LocalNode
 import gevent
@@ -25,6 +26,16 @@ def test_args_debug(self):
         r = PsDashRunner(args=['-d'])
         self.assertTrue(r.app.debug)
 
+    def test_default_args_dont_override_config(self):
+        _, filename = tempfile.mkstemp()
+        with open(filename, "w") as f:
+            f.write("PSDASH_LOGS = ['/var/log/boot.log', '/var/log/dmesg']\n")
+            f.flush()
+        os.environ['PSDASH_CONFIG'] = filename
+        r = PsDashRunner()
+        self.assertEquals(r.app.config['PSDASH_LOGS'], ['/var/log/boot.log', '/var/log/dmesg'])
+        del os.environ['PSDASH_CONFIG']
+
     def test_reload_logs(self):
         _, filename = tempfile.mkstemp()
         r = PsDashRunner(args=['-l', filename])