Use argparse to parse command arguments

This commit also allows users to specify all the config values through
the command line.

Fix #154.

config

@@ -45,7 +45,7 @@
 # SSL Protocol used. See python's ssl module for available values
 #protocol = PROTOCOL_SSLv23
 
-# Ciphers available. See python's ssl module for available ciphers
+# Available ciphers. See python's ssl module for available ciphers
 #ciphers =
 
 # Reverse DNS to resolve client address in logs
@@ -106,11 +106,16 @@
 # Sync all changes to disk during requests. (This can impair performance.)
 # Disabling it increases the risk of data loss, when the system crashes or
 # power fails!
-#fsync = True
+#filesystem_fsync = True
+
+# Close the lock file when no more clients are waiting.
+# This option is not very useful in general, but on Windows files that are
+# opened cannot be deleted.
+#filesystem_close_lock_file = False
 
 # Command that is run after changes to storage
-#hook =
 # Example: git add -A && (git diff --cached --quiet || git commit -m "Changes by "%(user)s)
+#hook =
 
 
 [logging]
@@ -120,8 +125,10 @@
 # For more information about the syntax of the configuration file, see:
 # http://docs.python.org/library/logging.config.html
 #config = /etc/radicale/logging
+
 # Set the default logging level to debug
 #debug = False
+
 # Store all environment variables (including those set in the shell)
 #full_environment = False
 

radicale/__init__.py

@@ -389,7 +389,7 @@ class Application:
             status = client.UNAUTHORIZED
             realm = self.configuration.get("server", "realm")
             headers = dict(headers)
-            headers.update ({
+            headers.update({
                 "WWW-Authenticate":
                 "Basic realm=\"%s\"" % realm})
 

radicale/__main__.py

@@ -22,8 +22,8 @@ from a python programme with ``radicale.__main__.run()``.
 
 """
 
+import argparse
 import atexit
-import optparse
 import os
 import select
 import signal
@@ -36,74 +36,47 @@ from . import (
   VERSION, Application, RequestHandler, ThreadedHTTPServer,
   ThreadedHTTPSServer, config, log)
 
-opt_dict = {
-    '--server-daemon': {
-        'help': 'launch as daemon',
-        'aliases': ['-d', '--daemon']},
-    '--server-pid': {
-        'help': 'set PID filename for daemon mode',
-        'aliases': ['-p', '--pid']},
-    '--server-hosts': {
-        'help': 'set server hostnames and ports',
-        'aliases': ['-H', '--hosts'],
-    },
-    '--server-ssl': {
-        'help': 'use SSL connection',
-        'aliases': ['-s', '--ssl'],
-    },
-    '--server-key': {
-        'help': 'set private key file',
-        'aliases': ['-k', '--key']
-    },
-    '--server-certificate': {
-        'help': 'set certificate file',
-        'aliases': ['-c', '--certificate']
-    },
-    '--logging-debug': {
-        'help': 'print debug informations',
-        'aliases': ['-D', '--debug']
-    }
-}
-
 
 def run():
     """Run Radicale as a standalone server."""
-    # Get command-line options
-    parser = optparse.OptionParser(version=VERSION)
-    for section, values in config.INITIAL_CONFIG.items():
-        group = optparse.OptionGroup(parser, section)
-        for option, default_value in values.items():
-            long_name = '--{0}-{1}'.format(
-                section, option.replace('_', '-'))
-            kwargs = {}
-            args = [long_name]
-            if default_value.lower() in ('true', 'false'):
-                kwargs['action'] = 'store_true'
-            if long_name in opt_dict:
-                args.extend(opt_dict[long_name].get('aliases'))
-                opt_dict[long_name].pop('aliases')
-                kwargs.update(opt_dict[long_name])
-            group.add_option(*args, **kwargs)
-        if section == 'server':
-            group.add_option(
-                "-f", "--foreground", action="store_false",
-                dest="server_daemon",
-                help="launch in foreground (opposite of --daemon)")
-            group.add_option(
-                "-S", "--no-ssl", action="store_false", dest="server_ssl",
-                help="do not use SSL connection (opposite of --ssl)")
-
-        parser.add_option_group(group)
+    # Get command-line arguments
+    parser = argparse.ArgumentParser(usage="radicale [OPTIONS]")
 
-    parser.add_option(
-        "-C", "--config",
-        help="use a specific configuration file")
+    parser.add_argument("--version", action="version", version=VERSION)
+    parser.add_argument(
+        "-C", "--config", help="use a specific configuration file")
 
-    options = parser.parse_args()[0]
-
-    if options.config:
+    groups = {}
+    for section, values in config.INITIAL_CONFIG.items():
+        group = parser.add_argument_group(section)
+        groups[group] = []
+        for option, data in values.items():
+            kwargs = data.copy()
+            long_name = "--{0}-{1}".format(
+                section, option.replace("_", "-"))
+            args = kwargs.pop("aliases", [])
+            args.append(long_name)
+            kwargs["dest"] = "{0}_{1}".format(section, option)
+            groups[group].append(kwargs["dest"])
+
+            if kwargs.pop("value") in ("True", "False"):
+                kwargs["action"] = "store_const"
+                kwargs["const"] = "True"
+                opposite_args = kwargs.pop("opposite", [])
+                opposite_args.append("--no{0}".format(long_name[1:]))
+                group.add_argument(*args, **kwargs)
+
+                kwargs["const"] = "False"
+                kwargs["help"] = "do not {0} (opposite of {1})".format(
+                    kwargs["help"], long_name)
+                group.add_argument(*opposite_args, **kwargs)
+            else:
+                group.add_argument(*args, **kwargs)
+
+    args = parser.parse_args()
+    if args.config:
         configuration = config.load()
-        configuration_found = configuration.read(options.config)
+        configuration_found = configuration.read(args.config)
     else:
         configuration_paths = [
             "/etc/radicale/config",
@@ -113,16 +86,13 @@ def run():
         configuration = config.load(configuration_paths)
         configuration_found = True
 
-    # Update Radicale configuration according to options
-    for group in parser.option_groups:
+    # Update Radicale configuration according to arguments
+    for group, actions in groups.items():
         section = group.title
-        for option in group.option_list:
-            key = option.dest
-            config_key = key.split('_', 1)[1]
-            if key:
-                value = getattr(options, key)
-                if value is not None:
-                    configuration.set(section, config_key, str(value))
+        for action in actions:
+            value = getattr(args, action)
+            if value is not None:
+                configuration.set(section, action.split('_', 1)[1], value)
 
     # Start logging
     filename = os.path.expanduser(configuration.get("logging", "config"))
@@ -131,7 +101,7 @@ def run():
 
     # Log a warning if the configuration file of the command line is not found
     if not configuration_found:
-        logger.warning("Configuration file '%s' not found" % options.config)
+        logger.warning("Configuration file '%s' not found" % args.config)
 
     try:
         serve(configuration, logger)

radicale/config.py

@@ -30,51 +30,122 @@ from configparser import RawConfigParser as ConfigParser
 # Default configuration
 INITIAL_CONFIG = OrderedDict([
     ("server", OrderedDict([
-        ("hosts", "0.0.0.0:5232"),
-        ("daemon", "False"),
-        ("pid", ""),
-        ("max_connections", "20"),
-        ("max_content_length", "10000000"),
-        ("timeout", "10"),
-        ("ssl", "False"),
-        ("certificate", "/etc/apache2/ssl/server.crt"),
-        ("key", "/etc/apache2/ssl/server.key"),
-        ("protocol", "PROTOCOL_SSLv23"),
-        ("ciphers", ""),
-        ("dns_lookup", "True"),
-        ("base_prefix", "/"),
-        ("can_skip_base_prefix", "False"),
-        ("realm", "Radicale - Password Required")])),
+        ("hosts", {
+            "value": "0.0.0.0:5232",
+            "help": "set server hostnames including ports",
+            "aliases": ["-H", "--hosts"]}),
+        ("daemon", {
+            "value": "False",
+            "help": "launch as daemon",
+            "aliases": ["-d", "--daemon"],
+            "opposite": ["-f", "--foreground"]}),
+        ("pid", {
+            "value": "",
+            "help": "set PID filename for daemon mode",
+            "aliases": ["-p", "--pid"]}),
+        ("max_connections", {
+            "value": "20",
+            "help": "maximum number of parallel connections"}),
+        ("max_content_length", {
+            "value": "10000000",
+            "help": "maximum size of request body in bytes"}),
+        ("timeout", {
+            "value": "10",
+            "help": "socket timeout"}),
+        ("ssl", {
+            "value": "False",
+            "help": "use SSL connection",
+            "aliases": ["-s", "--ssl"],
+            "opposite": ["-S", "--no-ssl"]}),
+        ("certificate", {
+            "value": "/etc/apache2/ssl/server.crt",
+            "help": "set certificate file",
+            "aliases": ["-c", "--certificate"]}),
+        ("key", {
+            "value": "/etc/apache2/ssl/server.key",
+            "help": "set private key file",
+            "aliases": ["-k", "--key"]}),
+        ("protocol", {
+            "value": "PROTOCOL_SSLv23",
+            "help": "SSL protocol used"}),
+        ("ciphers", {
+            "value": "",
+            "help": "available ciphers"}),
+        ("dns_lookup", {
+            "value": "True",
+            "help": "use reverse DNS to resolve client address in logs"}),
+        ("base_prefix", {
+            "value": "/",
+            "help": "root URL of Radicale, starting and ending with a slash"}),
+        ("can_skip_base_prefix", {
+            "value": "False",
+            "help": "allow URLs cleaned by a HTTP server"}),
+        ("realm", {
+            "value": "Radicale - Password Required",
+            "help": "message displayed when a password is needed"})])),
     ("encoding", OrderedDict([
-        ("request", "utf-8"),
-        ("stock", "utf-8")])),
+        ("request", {
+            "value": "utf-8",
+            "help": "encoding for responding requests"}),
+        ("stock", {
+            "value": "utf-8",
+            "help": "encoding for storing local collections"})])),
     ("auth", OrderedDict([
-        ("type", "None"),
-        ("htpasswd_filename", "/etc/radicale/users"),
-        ("htpasswd_encryption", "crypt")])),
+        ("type", {
+            "value": "None",
+            "help": "authentication method"}),
+        ("htpasswd_filename", {
+            "value": "/etc/radicale/users",
+            "help": "htpasswd filename"}),
+        ("htpasswd_encryption", {
+            "value": "crypt",
+            "help": "htpasswd encryption method"})])),
     ("rights", OrderedDict([
-        ("type", "None"),
-        ("file", "~/.config/radicale/rights")])),
+        ("type", {
+            "value": "None",
+            "help": "rights backend"}),
+        ("file", {
+            "value": "~/.config/radicale/rights",
+            "help": "file for rights management from_file"})])),
     ("storage", OrderedDict([
-        ("type", "multifilesystem"),
-        ("filesystem_folder", os.path.expanduser(
-            "~/.config/radicale/collections")),
-        ("fsync", "True"),
-        ("hook", ""),
-        ("close_lock_file", "False")])),
+        ("type", {
+            "value": "multifilesystem",
+            "help": "storage backend"}),
+        ("filesystem_folder", {
+            "value": os.path.expanduser(
+                "~/.config/radicale/collections"),
+            "help": "file for rights management from_file"}),
+        ("filesystem_fsync", {
+            "value": "True",
+            "help": "sync all changes to filesystem during requests"}),
+        ("filesystem_close_lock_file", {
+            "value": "False",
+            "help": "close the lock file when no more clients are waiting"}),
+        ("hook", {
+            "value": "",
+            "help": "command that is run after changes to storage"})])),
     ("logging", OrderedDict([
-        ("config", "/etc/radicale/logging"),
-        ("debug", "False"),
-        ("full_environment", "False"),
-        ("mask_passwords", "True")]))])
+        ("config", {
+            "value": "/etc/radicale/logging",
+            "help": "logging configuration file"}),
+        ("debug", {
+            "value": "False",
+            "help": "print debug information",
+            "aliases": ["-D", "--debug"]}),
+        ("full_environment", {
+            "value": "False",
+            "help": "store all environment variables"}),
+        ("mask_passwords", {
+            "value": "True",
+            "help": "mask passwords in logs"})]))])
 
 
 def load(paths=(), extra_config=None):
     config = ConfigParser()
     for section, values in INITIAL_CONFIG.items():
         config.add_section(section)
-        for key, value in values.items():
-            config.set(section, key, value)
+        for key, data in values.items():
+            config.set(section, key, data["value"])
     if extra_config:
         for section, values in extra_config.items():
             for key, value in values.items():

radicale/storage.py

@@ -389,7 +389,7 @@ class Collection(BaseCollection):
             delete=False, prefix=".Radicale.tmp-", newline=newline)
         try:
             yield tmp
-            if self.configuration.getboolean("storage", "fsync"):
+            if self.configuration.getboolean("storage", "filesystem_fsync"):
                 if os.name == "posix" and hasattr(fcntl, "F_FULLFSYNC"):
                     fcntl.fcntl(tmp.fileno(), fcntl.F_FULLFSYNC)
                 else:
@@ -418,7 +418,7 @@ class Collection(BaseCollection):
         This only works on POSIX and does nothing on other systems.
 
         """
-        if not cls.configuration.getboolean("storage", "fsync"):
+        if not cls.configuration.getboolean("storage", "filesystem_fsync"):
             return
         if os.name == "posix":
             fd = os.open(path, 0)
@@ -550,13 +550,15 @@ class Collection(BaseCollection):
                         new_collection = vobject.iCalendar()
                         for item in items:
                             new_collection.add(item)
-                        href = self._find_available_file_name(vobject_items.get)
+                        href = self._find_available_file_name(
+                            vobject_items.get)
                         vobject_items[href] = new_collection
                     self.upload_all_nonatomic(vobject_items)
                 elif props.get("tag") == "VCARD":
                     vobject_items = {}
                     for card in collection:
-                        href = self._find_available_file_name(vobject_items.get)
+                        href = self._find_available_file_name(
+                            vobject_items.get)
                         vobject_items[href] = card
                     self.upload_all_nonatomic(vobject_items)
 
@@ -583,7 +585,7 @@ class Collection(BaseCollection):
             fs.append(open(path, "w", encoding=self.encoding, newline=""))
             fs[-1].write(item.serialize())
         fsync_fn = lambda fd: None
-        if self.configuration.getboolean("storage", "fsync"):
+        if self.configuration.getboolean("storage", "filesystem_fsync"):
             if os.name == "posix" and hasattr(fcntl, "F_FULLFSYNC"):
                 fsync_fn = lambda fd: fcntl.fcntl(fd, fcntl.F_FULLFSYNC)
             else:
@@ -811,7 +813,8 @@ class Collection(BaseCollection):
                     cls._lock_file_locked = False
                 if cls._waiters:
                     cls._waiters[0].notify()
-                if (cls.configuration.getboolean("storage", "close_lock_file")
+                if (cls.configuration.getboolean(
+                        "storage", "filesystem_close_lock_file")
                         and cls._readers == 0 and not cls._waiters):
                     cls._lock_file.close()
                     cls._lock_file = None

radicale/tests/test_base.py

@@ -792,15 +792,15 @@ class BaseRequestsMixIn:
 
     def test_fsync(self):
         """Create a directory and file with syncing enabled."""
-        self.configuration.set("storage", "fsync", "True")
+        self.configuration.set("storage", "filesystem_fsync", "True")
         status, headers, answer = self.request("MKCALENDAR", "/calendar.ics/")
         assert status == 201
 
     def test_hook(self):
         """Run hook."""
         self.configuration.set(
-            "storage", "hook", "mkdir %s" % os.path.join("collection-root",
-                                                         "created_by_hook"))
+            "storage", "hook", "mkdir %s" % os.path.join(
+                "collection-root", "created_by_hook"))
         status, headers, answer = self.request("MKCOL", "/calendar.ics/")
         assert status == 201
         status, headers, answer = self.request("PROPFIND", "/created_by_hook/")
@@ -809,8 +809,8 @@ class BaseRequestsMixIn:
     def test_hook_read_access(self):
         """Verify that hook is not run for read accesses."""
         self.configuration.set(
-            "storage", "hook", "mkdir %s" % os.path.join("collection-root",
-                                                         "created_by_hook"))
+            "storage", "hook", "mkdir %s" % os.path.join(
+                "collection-root", "created_by_hook"))
         status, headers, answer = self.request("GET", "/")
         assert status == 200
         status, headers, answer = self.request("GET", "/created_by_hook/")
@@ -828,8 +828,8 @@ class BaseRequestsMixIn:
     def test_hook_principal_collection_creation(self):
         """Verify that the hooks runs when a new user is created."""
         self.configuration.set(
-            "storage", "hook", "mkdir %s" % os.path.join("collection-root",
-                                                         "created_by_hook"))
+            "storage", "hook", "mkdir %s" % os.path.join(
+                "collection-root", "created_by_hook"))
         status, headers, answer = self.request("GET", "/", REMOTE_USER="user")
         assert status == 200
         status, headers, answer = self.request("PROPFIND", "/created_by_hook/")
@@ -852,7 +852,8 @@ class BaseRequestsMixIn:
         status, headers, answer = self.request("GET", "/")
         assert headers.get("test") == "123"
         # Test if header is set on failure
-        status, headers, answer = self.request("GET", "/.well-known/does not exist")
+        status, headers, answer = self.request(
+            "GET", "/.well-known/does not exist")
         assert headers.get("test") == "123"
 
 
@@ -867,7 +868,7 @@ class BaseFileSystemTest(BaseTest):
         self.colpath = tempfile.mkdtemp()
         self.configuration.set("storage", "filesystem_folder", self.colpath)
         # Disable syncing to disk for better performance
-        self.configuration.set("storage", "fsync", "False")
+        self.configuration.set("storage", "filesystem_fsync", "False")
         # Required on Windows, doesn't matter on Unix
         self.configuration.set("storage", "close_lock_file", "True")
         self.application = Application(self.configuration, self.logger)