Merge branch 'develop' into rav/cleanup_metrics

1 files changed, 69 insertions, 43 deletions

diff --git a/synapse/config/_base.py b/synapse/config/_base.py
index f7d7f153bb..36e9c04cee 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -196,6 +196,12 @@ class Config(object):
 
     @classmethod
     def load_config(cls, description, argv):
+        """Parse the commandline and config files
+
+        Doesn't support config-file-generation: used by the worker apps.
+
+        Returns: Config object.
+        """
         config_parser = argparse.ArgumentParser(description=description)
         config_parser.add_argument(
             "-c",
@@ -222,9 +228,10 @@ class Config(object):
 
         config_files = find_config_files(search_paths=config_args.config_path)
 
-        obj.read_config_files(
-            config_files, keys_directory=config_args.keys_directory, generate_keys=False
+        config_dict = obj.read_config_files(
+            config_files, keys_directory=config_args.keys_directory
         )
+        obj.parse_config_dict(config_dict)
 
         obj.invoke_all("read_arguments", config_args)
 
@@ -232,6 +239,12 @@ class Config(object):
 
     @classmethod
     def load_or_generate_config(cls, description, argv):
+        """Parse the commandline and config files
+
+        Supports generation of config files, so is used for the main homeserver app.
+
+        Returns: Config object, or None if --generate-config or --generate-keys was set
+        """
         config_parser = argparse.ArgumentParser(add_help=False)
         config_parser.add_argument(
             "-c",
@@ -241,37 +254,43 @@ class Config(object):
             help="Specify config file. Can be given multiple times and"
             " may specify directories containing *.yaml files.",
         )
-        config_parser.add_argument(
+
+        generate_group = config_parser.add_argument_group("Config generation")
+        generate_group.add_argument(
             "--generate-config",
             action="store_true",
-            help="Generate a config file for the server name",
+            help="Generate a config file, then exit.",
         )
-        config_parser.add_argument(
+        generate_group.add_argument(
+            "--generate-missing-configs",
+            "--generate-keys",
+            action="store_true",
+            help="Generate any missing additional config files, then exit.",
+        )
+        generate_group.add_argument(
+            "-H", "--server-name", help="The server name to generate a config file for."
+        )
+        generate_group.add_argument(
             "--report-stats",
             action="store",
-            help="Whether the generated config reports anonymized usage statistics",
+            help="Whether the generated config reports anonymized usage statistics.",
             choices=["yes", "no"],
         )
-        config_parser.add_argument(
-            "--generate-keys",
-            action="store_true",
-            help="Generate any missing key files then exit",
-        )
-        config_parser.add_argument(
+        generate_group.add_argument(
+            "--config-directory",
             "--keys-directory",
             metavar="DIRECTORY",
-            help="Used with 'generate-*' options to specify where files such as"
-            " signing keys should be stored, unless explicitly"
-            " specified in the config.",
-        )
-        config_parser.add_argument(
-            "-H", "--server-name", help="The server name to generate a config file for"
+            help=(
+                "Specify where additional config files such as signing keys and log"
+                " config should be stored. Defaults to the same directory as the main"
+                " config file."
+            ),
         )
         config_args, remaining_args = config_parser.parse_known_args(argv)
 
         config_files = find_config_files(search_paths=config_args.config_path)
 
-        generate_keys = config_args.generate_keys
+        generate_missing_configs = config_args.generate_missing_configs
 
         obj = cls()
 
@@ -284,13 +303,14 @@ class Config(object):
             if not config_files:
                 config_parser.error(
                     "Must supply a config file.\nA config file can be automatically"
-                    " generated using \"--generate-config -H SERVER_NAME"
-                    " -c CONFIG-FILE\""
+                    ' generated using "--generate-config -H SERVER_NAME'
+                    ' -c CONFIG-FILE"'
                 )
             (config_path,) = config_files
             if not cls.path_exists(config_path):
-                if config_args.keys_directory:
-                    config_dir_path = config_args.keys_directory
+                print("Generating config file %s" % (config_path,))
+                if config_args.config_directory:
+                    config_dir_path = config_args.config_directory
                 else:
                     config_dir_path = os.path.dirname(config_path)
                 config_dir_path = os.path.abspath(config_dir_path)
@@ -313,9 +333,7 @@ class Config(object):
                 if not cls.path_exists(config_dir_path):
                     os.makedirs(config_dir_path)
                 with open(config_path, "w") as config_file:
-                    config_file.write(
-                        "# vim:ft=yaml\n\n"
-                    )
+                    config_file.write("# vim:ft=yaml\n\n")
                     config_file.write(config_str)
 
                 config = yaml.safe_load(config_str)
@@ -333,12 +351,12 @@ class Config(object):
             else:
                 print(
                     (
-                        "Config file %r already exists. Generating any missing key"
+                        "Config file %r already exists. Generating any missing config"
                         " files."
                     )
                     % (config_path,)
                 )
-                generate_keys = True
+                generate_missing_configs = True
 
         parser = argparse.ArgumentParser(
             parents=[config_parser],
@@ -352,37 +370,43 @@ class Config(object):
         if not config_files:
             config_parser.error(
                 "Must supply a config file.\nA config file can be automatically"
-                " generated using \"--generate-config -H SERVER_NAME"
-                " -c CONFIG-FILE\""
+                ' generated using "--generate-config -H SERVER_NAME'
+                ' -c CONFIG-FILE"'
             )
 
-        obj.read_config_files(
-            config_files,
-            keys_directory=config_args.keys_directory,
-            generate_keys=generate_keys,
+        config_dict = obj.read_config_files(
+            config_files, keys_directory=config_args.config_directory
         )
 
-        if generate_keys:
+        if generate_missing_configs:
+            obj.generate_missing_files(config_dict)
             return None
 
+        obj.parse_config_dict(config_dict)
         obj.invoke_all("read_arguments", args)
 
         return obj
 
-    def read_config_files(self, config_files, keys_directory=None, generate_keys=False):
+    def read_config_files(self, config_files, keys_directory=None):
+        """Read the config files into a dict
+
+        Returns: dict
+        """
         if not keys_directory:
             keys_directory = os.path.dirname(config_files[-1])
 
         self.config_dir_path = os.path.abspath(keys_directory)
 
+        # first we read the config files into a dict
         specified_config = {}
         for config_file in config_files:
             yaml_config = self.read_config_file(config_file)
             specified_config.update(yaml_config)
 
+        # not all of the options have sensible defaults in code, so we now need to
+        # generate a default config file suitable for the specified server name...
         if "server_name" not in specified_config:
             raise ConfigError(MISSING_SERVER_NAME)
-
         server_name = specified_config["server_name"]
         config_string = self.generate_config(
             config_dir_path=self.config_dir_path,
@@ -390,7 +414,11 @@ class Config(object):
             server_name=server_name,
             generate_secrets=False,
         )
+
+        # ... and read it into a base config dict ...
         config = yaml.safe_load(config_string)
+
+        # ... and finally, overlay it with the actual configuration.
         config.pop("log_config")
         config.update(specified_config)
 
@@ -400,16 +428,14 @@ class Config(object):
                 + "\n"
                 + MISSING_REPORT_STATS_SPIEL
             )
-
-        if generate_keys:
-            self.invoke_all("generate_files", config)
-            return
-
-        self.parse_config_dict(config)
+        return config
 
     def parse_config_dict(self, config_dict):
         self.invoke_all("read_config", config_dict)
 
+    def generate_missing_files(self, config_dict):
+        self.invoke_all("generate_files", config_dict)
+
 
 def find_config_files(search_paths):
     """Finds config files using a list of search paths. If a path is a file