Merge branch 'develop' into madlittlemods/relocate-snowflake-config-validation

Conflicts:
	synapse/app/homeserver.py
	tests/config/test_registration_config.py

Author: MadLittleMods

changelog.d/19015.misc

@@ -0,0 +1 @@
+Split homeserver creation (`create_homeserver`) and setup (`setup`).

synapse/app/homeserver.py

@@ -83,6 +83,10 @@ def gz_wrap(r: Resource) -> Resource:
 
 
 class SynapseHomeServer(HomeServer):
+    """
+    Homeserver class for the main Synapse process.
+    """
+
     DATASTORE_CLASS = DataStore
 
     def _listener_http(
@@ -345,23 +349,17 @@ def load_or_generate_config(argv_options: List[str]) -> HomeServerConfig:
     return config
 
 
-def setup(
+def create_homeserver(
     config: HomeServerConfig,
     reactor: Optional[ISynapseReactor] = None,
-    freeze: bool = True,
 ) -> SynapseHomeServer:
     """
-    Create and setup the main Synapse homeserver instance given a configuration.
+    Create a homeserver instance for the Synapse main process.
 
     Args:
         config: The configuration for the homeserver.
         reactor: Optionally provide a reactor to use. Can be useful in different
             scenarios that you want control over the reactor, such as tests.
-        freeze: whether to freeze the homeserver base objects in the garbage collector.
-            May improve garbage collection performance by marking objects with an effectively
-            static lifetime as frozen so they don't need to be considered for cleanup.
-            If you ever want to `shutdown` the homeserver, this needs to be
-            False otherwise the homeserver cannot be garbage collected after `shutdown`.
 
     Returns:
         A homeserver instance.
@@ -380,24 +378,50 @@ def setup(
         synapse.metrics.MIN_TIME_BETWEEN_GCS = config.server.gc_seconds
 
     hs = SynapseHomeServer(
-        config.server.server_name,
+        hostname=config.server.server_name,
         config=config,
         reactor=reactor,
     )
 
-    setup_logging(hs, config, use_worker_options=False)
+    return hs
 
-    # Start the tracer
-    init_tracer(hs)  # noqa
 
+def setup(
+    hs: SynapseHomeServer,
+    *,
+    freeze: bool = True,
+) -> None:
+    """
+    Setup a Synapse homeserver instance given a configuration.
+
+    Args:
+        hs: The homeserver to setup.
+        freeze: whether to freeze the homeserver base objects in the garbage collector.
+            May improve garbage collection performance by marking objects with an effectively
+            static lifetime as frozen so they don't need to be considered for cleanup.
+            If you ever want to `shutdown` the homeserver, this needs to be
+            False otherwise the homeserver cannot be garbage collected after `shutdown`.
+
+    Returns:
+        A homeserver instance.
+    """
+
+    setup_logging(hs, hs.config, use_worker_options=False)
+
+    # Log after we've configured logging.
     logger.info("Setting up server")
 
+    # Start the tracer
+    init_tracer(hs)  # noqa
+
     try:
         hs.setup()
     except Exception as e:
         handle_startup_exception(e)
 
-    async def start() -> None:
+    async def _start_when_reactor_running() -> None:
+        # TODO: Feels like this should be moved somewhere else.
+        #
         # Load the OIDC provider metadatas, if OIDC is enabled.
         if hs.config.oidc.oidc_enabled:
             oidc = hs.get_oidc_handler()
@@ -406,21 +430,31 @@ async def start() -> None:
 
         await _base.start(hs, freeze)
 
+        # TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (not
+        # `HomeServer.start_background_tasks`) (this way it matches the behavior of only
+        # running on `main`)
         hs.get_datastores().main.db_pool.updates.start_doing_background_updates()
 
-    register_start(hs, start)
+    # Register a callback to be invoked once the reactor is running
+    register_start(hs, _start_when_reactor_running)
 
-    return hs
 
+def start_reactor(
+    config: HomeServerConfig,
+) -> None:
+    """
+    Start the reactor (Twisted event-loop).
 
-def run(hs: HomeServer) -> None:
+    Args:
+        config: The configuration for the homeserver.
+    """
     _base.start_reactor(
         "synapse-homeserver",
-        soft_file_limit=hs.config.server.soft_file_limit,
-        gc_thresholds=hs.config.server.gc_thresholds,
-        pid_file=hs.config.server.pid_file,
-        daemonize=hs.config.server.daemonize,
-        print_pidfile=hs.config.server.print_pidfile,
+        soft_file_limit=config.server.soft_file_limit,
+        gc_thresholds=config.server.gc_thresholds,
+        pid_file=config.server.pid_file,
+        daemonize=config.server.daemonize,
+        print_pidfile=config.server.print_pidfile,
         logger=logger,
     )
 
@@ -431,13 +465,14 @@ def main() -> None:
     with LoggingContext(name="main", server_name=homeserver_config.server.server_name):
         # check base requirements
         check_requirements()
-        hs = setup(homeserver_config)
+        hs = create_homeserver(homeserver_config)
+        setup(hs)
 
         # redirect stdio to the logs, if configured.
         if not hs.config.logging.no_redirect_stdio:
             redirect_stdio_to_logs()
 
-        run(hs)
+        start_reactor(homeserver_config)
 
 
 if __name__ == "__main__":

tests/app/test_homeserver_start.py

@@ -37,7 +37,13 @@ def test_wrong_start_caught(self) -> None:
         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):
+            # Do a normal homeserver creation and setup
             homeserver_config = synapse.app.homeserver.load_or_generate_config(
                 ["-c", self.config_file]
             )
-            synapse.app.homeserver.setup(homeserver_config)
+            # XXX: The error will be raised at this point
+            hs = synapse.app.homeserver.create_homeserver(homeserver_config)
+            # Continue with the setup. We don't expect this to run because we raised
+            # earlier, but in the future, the code could be refactored to raise the
+            # error in a different place.
+            synapse.app.homeserver.setup(hs)

tests/config/test_registration_config.py

@@ -118,10 +118,16 @@ def test_refuse_to_start_if_open_registration_and_no_verification(self) -> None:
 
         # Test that allowing open registration without verification raises an error
         with self.assertRaises(SystemExit):
+            # Do a normal homeserver creation and setup
             homeserver_config = synapse.app.homeserver.load_or_generate_config(
                 ["-c", self.config_file]
             )
-            synapse.app.homeserver.setup(homeserver_config)
+            # XXX: The error will be raised at this point
+            hs = synapse.app.homeserver.create_homeserver(homeserver_config)
+            # Continue with the setup. We don't expect this to run because we raised
+            # earlier, but in the future, the code could be refactored to raise the
+            # error in a different place.
+            synapse.app.homeserver.setup(hs)
 
     def test_load_config_error_if_open_registration_and_no_verification(self) -> None:
         """