From 5be7679dd9dec79b72a2c541ab61efbbd79e1d62 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 22 Sep 2025 14:53:02 -0500 Subject: [PATCH] Split loading config vs homeserver `setup` (#18933) This allows us to get access to `server_name` so we can use it when creating the `LoggingContext("main")` in the future (pre-requisite for https://github.com/element-hq/synapse/pull/18868). This also allows us more flexibility to parse config however we want and setup a Synapse homeserver. Like what we do in [Synapse Pro for Small Hosts](https://github.com/element-hq/synapse-small-hosts). Split out from https://github.com/element-hq/synapse/pull/18868 --- changelog.d/18933.misc | 1 + synapse/app/admin_cmd.py | 17 +++++++++----- synapse/app/appservice.py | 7 +++--- synapse/app/client_reader.py | 7 +++--- synapse/app/event_creator.py | 7 +++--- synapse/app/federation_reader.py | 7 +++--- synapse/app/federation_sender.py | 7 +++--- synapse/app/frontend_proxy.py | 7 +++--- synapse/app/generic_worker.py | 22 +++++++++++++++---- synapse/app/homeserver.py | 28 ++++++++++++++++++++---- synapse/app/media_repository.py | 7 +++--- synapse/app/pusher.py | 7 +++--- synapse/app/synchrotron.py | 7 +++--- synapse/app/user_dir.py | 7 +++--- synapse/config/_base.py | 8 +++++-- tests/app/test_homeserver_start.py | 5 ++++- tests/config/test_registration_config.py | 5 ++++- 17 files changed, 108 insertions(+), 48 deletions(-) create mode 100644 changelog.d/18933.misc diff --git a/changelog.d/18933.misc b/changelog.d/18933.misc new file mode 100644 index 000000000..3308d367e --- /dev/null +++ b/changelog.d/18933.misc @@ -0,0 +1 @@ +Split loading config from homeserver `setup`. diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index 7c9b94c65..c0c594577 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -24,7 +24,7 @@ import logging import os import sys import tempfile -from typing import List, Mapping, Optional, Sequence +from typing import List, Mapping, Optional, Sequence, Tuple from twisted.internet import defer, task @@ -256,7 +256,7 @@ class FileExfiltrationWriter(ExfiltrationWriter): return self.base_directory -def start(config_options: List[str]) -> None: +def load_config(argv_options: List[str]) -> Tuple[HomeServerConfig, argparse.Namespace]: parser = argparse.ArgumentParser(description="Synapse Admin Command") HomeServerConfig.add_arguments_to_parser(parser) @@ -282,11 +282,15 @@ def start(config_options: List[str]) -> None: export_data_parser.set_defaults(func=export_data_command) try: - config, args = HomeServerConfig.load_config_with_parser(parser, config_options) + config, args = HomeServerConfig.load_config_with_parser(parser, argv_options) except ConfigError as e: sys.stderr.write("\n" + str(e) + "\n") sys.exit(1) + return config, args + + +def start(config: HomeServerConfig, args: argparse.Namespace) -> None: if config.worker.worker_app is not None: assert config.worker.worker_app == "synapse.app.admin_cmd" @@ -325,7 +329,7 @@ def start(config_options: List[str]) -> None: # command. async def run() -> None: - with LoggingContext("command"): + with LoggingContext(name="command"): await _base.start(ss) await args.func(ss, args) @@ -337,5 +341,6 @@ def start(config_options: List[str]) -> None: if __name__ == "__main__": - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config, args = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config, args) diff --git a/synapse/app/appservice.py b/synapse/app/appservice.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/appservice.py +++ b/synapse/app/appservice.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/event_creator.py b/synapse/app/event_creator.py index f0d41ed22..351cf93b7 100644 --- a/synapse/app/event_creator.py +++ b/synapse/app/event_creator.py @@ -20,13 +20,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/federation_reader.py +++ b/synapse/app/federation_reader.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/federation_sender.py b/synapse/app/federation_sender.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/federation_sender.py +++ b/synapse/app/federation_sender.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/frontend_proxy.py b/synapse/app/frontend_proxy.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/frontend_proxy.py +++ b/synapse/app/frontend_proxy.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 0849f2505..afc6b85ea 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -310,13 +310,26 @@ class GenericWorkerServer(HomeServer): self.get_replication_command_handler().start_replication(self) -def start(config_options: List[str]) -> None: +def load_config(argv_options: List[str]) -> HomeServerConfig: + """ + Parse the commandline and config files (does not generate config) + + Args: + argv_options: The options passed to Synapse. Usually `sys.argv[1:]`. + + Returns: + Config object. + """ try: - config = HomeServerConfig.load_config("Synapse worker", config_options) + config = HomeServerConfig.load_config("Synapse worker", argv_options) except ConfigError as e: sys.stderr.write("\n" + str(e) + "\n") sys.exit(1) + return config + + +def start(config: HomeServerConfig) -> None: # For backwards compatibility let any of the old app names. assert config.worker.worker_app in ( "synapse.app.appservice", @@ -368,8 +381,9 @@ def start(config_options: List[str]) -> None: def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 54c41c0c2..163f7c70a 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -308,17 +308,21 @@ class SynapseHomeServer(HomeServer): logger.warning("Unrecognized listener type: %s", listener.type) -def setup(config_options: List[str]) -> SynapseHomeServer: +def load_or_generate_config(argv_options: List[str]) -> HomeServerConfig: """ + Parse the commandline and config files + + Supports generation of config files, so is used for the main homeserver app. + Args: - config_options_options: The options passed to Synapse. Usually `sys.argv[1:]`. + argv_options: The options passed to Synapse. Usually `sys.argv[1:]`. Returns: A homeserver instance. """ try: config = HomeServerConfig.load_or_generate_config( - "Synapse Homeserver", config_options + "Synapse Homeserver", argv_options ) except ConfigError as e: sys.stderr.write("\n") @@ -332,6 +336,20 @@ def setup(config_options: List[str]) -> SynapseHomeServer: # generating config files and shouldn't try to continue. sys.exit(0) + return config + + +def setup(config: HomeServerConfig) -> SynapseHomeServer: + """ + Create and setup a Synapse homeserver instance given a configuration. + + Args: + config: The configuration for the homeserver. + + Returns: + A homeserver instance. + """ + if config.worker.worker_app: raise ConfigError( "You have specified `worker_app` in the config but are attempting to start a non-worker " @@ -405,10 +423,12 @@ def run(hs: HomeServer) -> None: def main() -> None: + homeserver_config = load_or_generate_config(sys.argv[1:]) + with LoggingContext("main"): # check base requirements check_requirements() - hs = setup(sys.argv[1:]) + hs = setup(homeserver_config) # redirect stdio to the logs, if configured. if not hs.config.logging.no_redirect_stdio: diff --git a/synapse/app/media_repository.py b/synapse/app/media_repository.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/media_repository.py +++ b/synapse/app/media_repository.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/pusher.py b/synapse/app/pusher.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/pusher.py +++ b/synapse/app/pusher.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/synchrotron.py b/synapse/app/synchrotron.py index c85ce2869..95a253dbb 100644 --- a/synapse/app/synchrotron.py +++ b/synapse/app/synchrotron.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/app/user_dir.py b/synapse/app/user_dir.py index dadcc4877..b6385381b 100644 --- a/synapse/app/user_dir.py +++ b/synapse/app/user_dir.py @@ -21,13 +21,14 @@ import sys -from synapse.app.generic_worker import start +from synapse.app.generic_worker import load_config, start from synapse.util.logcontext import LoggingContext def main() -> None: - with LoggingContext("main"): - start(sys.argv[1:]) + homeserver_config = load_config(sys.argv[1:]) + with LoggingContext(name="main"): + start(homeserver_config) if __name__ == "__main__": diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 191253ddd..f3b6e9f88 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -646,12 +646,16 @@ class RootConfig: @classmethod def load_or_generate_config( - cls: Type[TRootConfig], description: str, argv: List[str] + cls: Type[TRootConfig], description: str, argv_options: List[str] ) -> Optional[TRootConfig]: """Parse the commandline and config files Supports generation of config files, so is used for the main homeserver app. + Args: + description: TODO + argv_options: The options passed to Synapse. Usually `sys.argv[1:]`. + Returns: Config object, or None if --generate-config or --generate-keys was set """ @@ -747,7 +751,7 @@ class RootConfig: ) cls.invoke_all_static("add_arguments", parser) - config_args = parser.parse_args(argv) + config_args = parser.parse_args(argv_options) config_files = find_config_files(search_paths=config_args.config_path) diff --git a/tests/app/test_homeserver_start.py b/tests/app/test_homeserver_start.py index 9dc20800b..0d257c98a 100644 --- a/tests/app/test_homeserver_start.py +++ b/tests/app/test_homeserver_start.py @@ -37,4 +37,7 @@ class HomeserverAppStartTestCase(ConfigFileTestCase): self.add_lines_to_config([" main:", " host: 127.0.0.1", " port: 1234"]) # Ensure that starting master process with worker config raises an exception with self.assertRaises(ConfigError): - synapse.app.homeserver.setup(["-c", self.config_file]) + homeserver_config = synapse.app.homeserver.load_or_generate_config( + ["-c", self.config_file] + ) + synapse.app.homeserver.setup(homeserver_config) diff --git a/tests/config/test_registration_config.py b/tests/config/test_registration_config.py index 7fd6df2f9..a8520c91d 100644 --- a/tests/config/test_registration_config.py +++ b/tests/config/test_registration_config.py @@ -112,4 +112,7 @@ class RegistrationConfigTestCase(ConfigFileTestCase): # Test that allowing open registration without verification raises an error with self.assertRaises(ConfigError): - synapse.app.homeserver.setup(["-c", self.config_file]) + homeserver_config = synapse.app.homeserver.load_or_generate_config( + ["-c", self.config_file] + ) + synapse.app.homeserver.setup(homeserver_config)