From 32e7c9e7f20b57dd081023ac42d6931a8da9b3a3 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Thu, 20 Jun 2019 19:32:02 +1000 Subject: Run Black. (#5482) --- synapse/config/_base.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'synapse/config/_base.py') diff --git a/synapse/config/_base.py b/synapse/config/_base.py index f7d7f153bb..8284aa4c6d 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -284,8 +284,8 @@ 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): @@ -313,9 +313,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) @@ -352,8 +350,8 @@ 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( -- cgit 1.4.1 From 37933a3bf88b6ddf0a01cb4b86e8645aca93ae0f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 21 Jun 2019 17:14:56 +0100 Subject: Improve logging when generating config files (#5510) Make it a bit clearer what's going on. --- changelog.d/5510.misc | 1 + synapse/config/_base.py | 3 ++- synapse/config/key.py | 1 + synapse/config/logger.py | 4 ++++ 4 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5510.misc (limited to 'synapse/config/_base.py') diff --git a/changelog.d/5510.misc b/changelog.d/5510.misc new file mode 100644 index 0000000000..4591a63d9d --- /dev/null +++ b/changelog.d/5510.misc @@ -0,0 +1 @@ +Improve logging when generating config files. diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 8284aa4c6d..4c6a684cb9 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -289,6 +289,7 @@ class Config(object): ) (config_path,) = config_files if not cls.path_exists(config_path): + print("Generating config file %s" % (config_path,)) if config_args.keys_directory: config_dir_path = config_args.keys_directory else: @@ -331,7 +332,7 @@ 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,) diff --git a/synapse/config/key.py b/synapse/config/key.py index 94a0f47ea4..21c4f5c51c 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -241,6 +241,7 @@ class KeyConfig(Config): signing_key_path = config["signing_key_path"] if not self.path_exists(signing_key_path): + print("Generating signing key file %s" % (signing_key_path,)) with open(signing_key_path, "w") as signing_key_file: key_id = "a_" + random_string(4) write_signing_keys(signing_key_file, (generate_signing_key(key_id),)) diff --git a/synapse/config/logger.py b/synapse/config/logger.py index a22655b125..9db2e087e4 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -137,6 +137,10 @@ class LoggingConfig(Config): log_config = config.get("log_config") if log_config and not os.path.exists(log_config): log_file = self.abspath("homeserver.log") + print( + "Generating log config file %s which will log to %s" + % (log_config, log_file) + ) with open(log_config, "w") as log_config_file: log_config_file.write(DEFAULT_LOG_CONFIG.substitute(log_file=log_file)) -- cgit 1.4.1 From 03cea2b0fed954e4558b7f0f652ae802c6e51604 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 21 Jun 2019 17:43:38 +0100 Subject: Refactor Config parser and add some comments. (#5511) Add some comments, and simplify `read_config_files`. --- changelog.d/5511.misc | 1 + synapse/config/_base.py | 49 +++++++++++++++++++++++++++++++++++-------------- 2 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 changelog.d/5511.misc (limited to 'synapse/config/_base.py') diff --git a/changelog.d/5511.misc b/changelog.d/5511.misc new file mode 100644 index 0000000000..c1f679b287 --- /dev/null +++ b/changelog.d/5511.misc @@ -0,0 +1 @@ +Refactor and clean up Config parser for maintainability. diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 4c6a684cb9..8f4cd4b562 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", @@ -355,33 +368,39 @@ class Config(object): ' -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=args.keys_directory ) if generate_keys: + 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, @@ -389,7 +408,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) @@ -399,16 +422,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 -- cgit 1.4.1 From e1a795758c6ba3b1147412eb16beb8c6698d4160 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 21 Jun 2019 18:50:43 +0100 Subject: Improve help and cmdline option names for --generate-config options (#5512) * group the arguments together into a group * add new names "--generate-missing-config" and "--config-directory" for existing cmdline options "--generate-keys" and "--keys-dir", which better reflect their purposes. --- changelog.d/5512.feature | 1 + synapse/config/_base.py | 50 +++++++++++++++++++++++++++--------------------- 2 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 changelog.d/5512.feature (limited to 'synapse/config/_base.py') diff --git a/changelog.d/5512.feature b/changelog.d/5512.feature new file mode 100644 index 0000000000..712878901b --- /dev/null +++ b/changelog.d/5512.feature @@ -0,0 +1 @@ +Improve help and cmdline option names for --generate-config options. diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 8f4cd4b562..36e9c04cee 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -254,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() @@ -303,8 +309,8 @@ class Config(object): (config_path,) = config_files if not cls.path_exists(config_path): print("Generating config file %s" % (config_path,)) - if config_args.keys_directory: - config_dir_path = config_args.keys_directory + 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) @@ -350,7 +356,7 @@ class Config(object): ) % (config_path,) ) - generate_keys = True + generate_missing_configs = True parser = argparse.ArgumentParser( parents=[config_parser], @@ -369,10 +375,10 @@ class Config(object): ) config_dict = obj.read_config_files( - config_files, keys_directory=args.keys_directory + config_files, keys_directory=config_args.config_directory ) - if generate_keys: + if generate_missing_configs: obj.generate_missing_files(config_dict) return None -- cgit 1.4.1