<p>Max has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/12134">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ctrl2cgi: fix broken config override<br><br>Previously command-line arguments without defaults took precedence over<br>config file variables while values from config file which had<br>command-line counterparts with default value were silently ignored.<br><br>Let's fix this by making config file values to be always preferred over<br>command-line equivalents.<br><br>The easiest way is to use TrapFactory as argparse namespace. This means<br>that some parameter values won't be known initially so logging is moved<br>directly to main.<br><br>Change-Id: I471b5a6497eadce6456e835233fdaba88a593324<br>Related: SYS#4399<br>---<br>M scripts/ctrl2cgi.py<br>1 file changed, 21 insertions(+), 39 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/python/osmo-python-tests refs/changes/34/12134/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/scripts/ctrl2cgi.py b/scripts/ctrl2cgi.py</span><br><span>index 1d6813d..89cbf50 100755</span><br><span>--- a/scripts/ctrl2cgi.py</span><br><span>+++ b/scripts/ctrl2cgi.py</span><br><span>@@ -22,7 +22,7 @@</span><br><span>  */</span><br><span> """</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-__version__ = "0.0.4" # bump this on every non-trivial change</span><br><span style="color: hsl(120, 100%, 40%);">+__version__ = "0.0.5" # bump this on every non-trivial change</span><br><span> </span><br><span> import argparse, os, logging, logging.handlers</span><br><span> import hashlib</span><br><span>@@ -90,7 +90,7 @@</span><br><span>         """</span><br><span>         Logging wrapper, calling super() is necessary not to break reconnection logic</span><br><span>         """</span><br><span style="color: hsl(0, 100%, 40%);">-        self.factory.log.info("Connected to CTRL@%s:%d" % (self.factory.host, self.factory.port))</span><br><span style="color: hsl(120, 100%, 40%);">+        self.factory.log.info("Connected to CTRL@%s:%d" % (self.factory.addr_ctrl, self.factory.port_ctrl))</span><br><span>         super(CTRL, self).connectionMade()</span><br><span> </span><br><span>     @defer.inlineCallbacks</span><br><span>@@ -120,25 +120,14 @@</span><br><span>     """</span><br><span>     Store CGI information so TRAP handler can use it for requests</span><br><span>     """</span><br><span style="color: hsl(0, 100%, 40%);">-    location = None</span><br><span style="color: hsl(0, 100%, 40%);">-    log = None</span><br><span style="color: hsl(0, 100%, 40%);">-    semaphore = None</span><br><span style="color: hsl(0, 100%, 40%);">-    client = None</span><br><span style="color: hsl(0, 100%, 40%);">-    host = None</span><br><span style="color: hsl(0, 100%, 40%);">-    port = None</span><br><span style="color: hsl(0, 100%, 40%);">-    secret_key = None</span><br><span style="color: hsl(0, 100%, 40%);">-    def __init__(self, host, port, proto, semaphore, log, location, secret_key):</span><br><span style="color: hsl(0, 100%, 40%);">-        self.host = host # for logging only,</span><br><span style="color: hsl(0, 100%, 40%);">-        self.port = port # seems to be no way to get it from ReconnectingClientFactory</span><br><span style="color: hsl(120, 100%, 40%);">+    def __init__(self, proto, log):</span><br><span style="color: hsl(120, 100%, 40%);">+        self.semaphore = defer.DeferredSemaphore(self.num_max_conn)</span><br><span>         self.log = log</span><br><span style="color: hsl(0, 100%, 40%);">-        self.semaphore = semaphore</span><br><span style="color: hsl(0, 100%, 40%);">-        self.location = location</span><br><span style="color: hsl(0, 100%, 40%);">-        self.secret_key = secret_key</span><br><span>         level = self.log.getEffectiveLevel()</span><br><span>         self.log.setLevel(logging.WARNING) # we do not need excessive debug from lower levels</span><br><span>         super(TrapFactory, self).__init__(proto, self.log)</span><br><span>         self.log.setLevel(level)</span><br><span style="color: hsl(0, 100%, 40%);">-        self.log.debug("Using IPA %s, CGI server: %s" % (Ctrl.version, self.location))</span><br><span style="color: hsl(120, 100%, 40%);">+        self.log.debug("Using Osmocom IPA library v%s" % Ctrl.version)</span><br><span> </span><br><span> </span><br><span> if __name__ == '__main__':</span><br><span>@@ -151,31 +140,24 @@</span><br><span>     p.add_argument('-o', '--output', action='store_true', help="Log to STDOUT in addition to SYSLOG")</span><br><span>     p.add_argument('-l', '--location', help="Location URL of the CGI server")</span><br><span>     p.add_argument('-s', '--secret-key', help="Secret key used to generate verification token")</span><br><span style="color: hsl(0, 100%, 40%);">-    p.add_argument('-c', '--config-file', help="Path Config file. Cmd line args override values in config file")</span><br><span style="color: hsl(0, 100%, 40%);">-    args = p.parse_args()</span><br><span style="color: hsl(120, 100%, 40%);">+    p.add_argument('-c', '--config-file', help="Path to config file (in INI format). Values from config file override command line options.")</span><br><span style="color: hsl(120, 100%, 40%);">+    args = p.parse_args(namespace=TrapFactory)</span><br><span> </span><br><span>     log = debug_init('CTRL2CGI', args.debug, args.output)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    location_cfgfile = None</span><br><span style="color: hsl(0, 100%, 40%);">-    secret_key_cfgfile = None</span><br><span style="color: hsl(0, 100%, 40%);">-    port_ctrl_cfgfile = None</span><br><span style="color: hsl(0, 100%, 40%);">-    addr_ctrl_cfgfile = None</span><br><span style="color: hsl(0, 100%, 40%);">-    num_max_conn_cfgfile = None</span><br><span style="color: hsl(0, 100%, 40%);">-    if args.config_file:</span><br><span style="color: hsl(0, 100%, 40%);">-        config = configparser.ConfigParser()</span><br><span style="color: hsl(0, 100%, 40%);">-        config.read(args.config_file)</span><br><span style="color: hsl(0, 100%, 40%);">-        if 'main' in config:</span><br><span style="color: hsl(0, 100%, 40%);">-            location_cfgfile = config['main'].get('location', None)</span><br><span style="color: hsl(0, 100%, 40%);">-            secret_key_cfgfile = config['main'].get('secret_key', None)</span><br><span style="color: hsl(0, 100%, 40%);">-            addr_ctrl_cfgfile = config['main'].get('addr_ctrl', None)</span><br><span style="color: hsl(0, 100%, 40%);">-            port_ctrl_cfgfile = config['main'].get('port_ctrl', None)</span><br><span style="color: hsl(0, 100%, 40%);">-            num_max_conn_cfgfile = config['main'].get('num_max_conn', None)</span><br><span style="color: hsl(0, 100%, 40%);">-    location = args.location if args.location is not None else location_cfgfile</span><br><span style="color: hsl(0, 100%, 40%);">-    secret_key = args.secret_key  if args.secret_key is not None else secret_key_cfgfile</span><br><span style="color: hsl(0, 100%, 40%);">-    addr_ctrl = args.addr_ctrl if args.addr_ctrl is not None else addr_ctrl_cfgfile</span><br><span style="color: hsl(0, 100%, 40%);">-    port_ctrl = args.port_ctrl if args.port_ctrl is not None else port_ctrl_cfgfile</span><br><span style="color: hsl(0, 100%, 40%);">-    num_max_conn = args.num_max_conn if args.num_max_conn is not None else num_max_conn_cfgfile</span><br><span style="color: hsl(120, 100%, 40%);">+    T = TrapFactory(Trap, log)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    log.info("CGI proxy %s starting with PID %d ..." % (__version__, os.getpid()))</span><br><span style="color: hsl(0, 100%, 40%);">-    reactor.connectTCP(addr_ctrl, port_ctrl, TrapFactory(addr_ctrl, port_ctrl, Trap, defer.DeferredSemaphore(num_max_conn), log, location, secret_key))</span><br><span style="color: hsl(120, 100%, 40%);">+    if args.config_file:</span><br><span style="color: hsl(120, 100%, 40%);">+        config = configparser.ConfigParser(interpolation=None)</span><br><span style="color: hsl(120, 100%, 40%);">+        config.read(args.config_file)</span><br><span style="color: hsl(120, 100%, 40%);">+        T.location = config['main'].get('location', T.location)</span><br><span style="color: hsl(120, 100%, 40%);">+        T.addr_ctrl = config['main'].get('addr_ctrl', T.addr_ctrl)</span><br><span style="color: hsl(120, 100%, 40%);">+        T.port_ctrl = config['main'].getint('port_ctrl', T.port_ctrl)</span><br><span style="color: hsl(120, 100%, 40%);">+        T.num_max_conn = config['main'].getint('num_max_conn', T.num_max_conn)</span><br><span style="color: hsl(120, 100%, 40%);">+        T.secret_key = config['main'].get('secret_key', T.secret_key)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    log.info("CGI proxy v%s starting with PID %d:" % (__version__, os.getpid()))</span><br><span style="color: hsl(120, 100%, 40%);">+    log.info("destination %s (concurrency %d)" % (T.location, T.num_max_conn))</span><br><span style="color: hsl(120, 100%, 40%);">+    log.info("connecting to %s:%d..." % (T.addr_ctrl, T.port_ctrl))</span><br><span style="color: hsl(120, 100%, 40%);">+    reactor.connectTCP(T.addr_ctrl, T.port_ctrl, T)</span><br><span>     reactor.run()</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12134">change 12134</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/12134"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: python/osmo-python-tests </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I471b5a6497eadce6456e835233fdaba88a593324 </div>
<div style="display:none"> Gerrit-Change-Number: 12134 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>