Misc refactor

main.py

@@ -19,6 +19,7 @@
 
 import humanize
 from docopt import docopt
+from prometheus_client import start_http_server
 
 import server
 from server.config import config
@@ -53,6 +54,10 @@ async def main():
         sys.platform
     )
 
+    if config.ENABLE_METRICS:
+        logger.info("Using prometheus on port: %i", config.METRICS_PORT)
+        start_http_server(config.METRICS_PORT)
+
     loop = asyncio.get_running_loop()
     done = loop.create_future()
 
@@ -212,7 +217,7 @@ def drain_handler(sig: int, frame):
     stop_time = time.perf_counter()
     logger.info(
         "Total server uptime: %s",
-        humanize.naturaldelta(stop_time - startup_time)
+        humanize.precisedelta(stop_time - startup_time)
     )
 
     if shutdown_time is not None:

server/__init__.py

@@ -90,8 +90,6 @@
 import time
 from typing import Optional
 
-from prometheus_client import start_http_server
-
 import server.metrics as metrics
 
 from .asyncio_extensions import map_suppress, synchronizedmethod
@@ -143,10 +141,6 @@
 
 logger = logging.getLogger("server")
 
-if config.ENABLE_METRICS:
-    logger.info("Using prometheus on port: %i", config.METRICS_PORT)
-    start_http_server(config.METRICS_PORT)
-
 
 class ServerInstance(object):
     """

server/config.py

@@ -174,10 +174,14 @@ def refresh(self) -> None:
                 with open(config_file) as f:
                     new_values.update(yaml.safe_load(f))
             except FileNotFoundError:
-                self._logger.info("No configuration file found at %s", config_file)
+                self._logger.warning(
+                    "No configuration file found at %s",
+                    config_file
+                )
             except TypeError:
                 self._logger.info(
-                    "Configuration file at %s appears to be empty", config_file
+                    "Configuration file at %s appears to be empty",
+                    config_file
                 )
 
         triggered_callback_keys = tuple(

server/game_service.py

@@ -215,10 +215,17 @@ def update_active_game_metrics(self):
                     rating_type_counter[(rating_type, state)]
                 )
 
+    @property
+    def all_games(self) -> ValuesView[Game]:
+        return self._games.values()
+
     @property
     def live_games(self) -> list[Game]:
-        return [game for game in self._games.values()
-                if game.state is GameState.LIVE]
+        return [
+            game
+            for game in self.all_games
+            if game.state is GameState.LIVE
+        ]
 
     @property
     def open_games(self) -> list[Game]:
@@ -233,17 +240,19 @@ def open_games(self) -> list[Game]:
 
         The client ignores everything "closed". This property fetches all such not-closed games.
         """
-        return [game for game in self._games.values()
-                if game.state is GameState.LOBBY or game.state is GameState.LIVE]
-
-    @property
-    def all_games(self) -> ValuesView[Game]:
-        return self._games.values()
+        return [
+            game
+            for game in self.all_games
+            if game.state in (GameState.LOBBY, GameState.LIVE)
+        ]
 
     @property
     def pending_games(self) -> list[Game]:
-        return [game for game in self._games.values()
-                if game.state is GameState.LOBBY or game.state is GameState.INITIALIZING]
+        return [
+            game
+            for game in self.all_games
+            if game.state in (GameState.LOBBY, GameState.INITIALIZING)
+        ]
 
     def remove_game(self, game: Game):
         if game.id in self._games:

server/games/game.py

@@ -168,7 +168,6 @@ def players(self) -> list[Player]:
         Depending on the state, it is either:
           - (LOBBY) The currently connected players
           - (LIVE) Players who participated in the game
-          - Empty list
         """
         if self.state is GameState.LOBBY:
             return self.get_connected_players()

server/ladder_service/violation_service.py

@@ -91,7 +91,7 @@ def register_violations(self, players: list[Player]):
             })
             extra_text = ""
             if violation.count > 1:
-                delta_text = humanize.naturaldelta(
+                delta_text = humanize.precisedelta(
                     violation.get_ban_expiration() - now
                 )
                 extra_text = f" You can queue again in {delta_text}"

server/lobbyconnection.py

@@ -181,41 +181,45 @@ async def on_message_received(self, message):
             handler = getattr(self, f"command_{cmd}")
             await handler(message)
 
-        except AuthenticationError as ex:
-            metrics.user_logins.labels("failure", ex.method).inc()
+        except AuthenticationError as e:
+            metrics.user_logins.labels("failure", e.method).inc()
             await self.send({
                 "command": "authentication_failed",
-                "text": ex.message
+                "text": e.message
             })
-        except BanError as ex:
+        except BanError as e:
             await self.send({
                 "command": "notice",
                 "style": "error",
-                "text": ex.message()
+                "text": e.message()
             })
-            await self.abort(ex.message())
-        except ClientError as ex:
-            self._logger.warning("Client error: %s", ex.message)
+            await self.abort(e.message())
+        except ClientError as e:
+            self._logger.warning("Client error: %s", e.message)
             await self.send({
                 "command": "notice",
                 "style": "error",
-                "text": ex.message
+                "text": e.message
             })
-            if not ex.recoverable:
-                await self.abort(ex.message)
-        except (KeyError, ValueError) as ex:
-            self._logger.exception(ex)
+            if not e.recoverable:
+                await self.abort(e.message)
+        except (KeyError, ValueError) as e:
+            self._logger.exception(e)
             await self.abort(f"Garbage command: {message}")
         except ConnectionError as e:
             # Propagate connection errors to the ServerContext error handler.
             raise e
         except DisabledError:
             # TODO: Respond with correlation uid for original message
             await self.send({"command": "disabled", "request": cmd})
-            self._logger.info("Ignoring disabled command: %s", cmd)
-        except Exception as ex:  # pragma: no cover
+            self._logger.info(
+                "Ignoring disabled command for %s: %s",
+                self.get_user_identifier(),
+                cmd
+            )
+        except Exception as e:  # pragma: no cover
             await self.send({"command": "invalid"})
-            self._logger.exception(ex)
+            self._logger.exception(e)
             await self.abort("Error processing command")
 
     async def command_ping(self, msg):
@@ -337,11 +341,10 @@ async def command_admin(self, message):
                         "Administrative action: %s closed game for %s",
                         self.player, player
                     )
-                    with contextlib.suppress(DisconnectedError):
-                        await player.send_message({
-                            "command": "notice",
-                            "style": "kill",
-                        })
+                    player.write_message({
+                        "command": "notice",
+                        "style": "kill",
+                    })
 
         elif action == "closelobby":
             if await self.player_service.has_permission_role(

tests/integration_tests/conftest.py

@@ -414,7 +414,7 @@ async def get_session(proto):
 async def connect_and_sign_in(
     credentials,
     lobby_server: ServerContext,
-):
+) -> tuple[int, int, Protocol]:
     proto = await connect_client(lobby_server)
     session = await get_session(proto)
     await perform_login(proto, credentials)

tests/integration_tests/test_server.py

@@ -41,7 +41,11 @@ async def test_login_timeout(lobby_server):
 async def test_server_deprecated_client(lobby_server):
     proto = await connect_client(lobby_server)
 
-    await proto.send_message({"command": "ask_session", "user_agent": "faf-client", "version": "0.0.0"})
+    await proto.send_message({
+        "command": "ask_session",
+        "user_agent": "faf-client",
+        "version": "0.0.0"
+    })
     msg = await proto.read_message()
 
     assert msg["command"] == "notice"

tests/unit_tests/conftest.py

@@ -127,7 +127,10 @@ def game_stats_service():
 
 def add_connected_player(game: Game, player):
     game.game_service.player_service[player.id] = player
-    gc = make_mock_game_connection(state=GameConnectionState.CONNECTED_TO_HOST, player=player)
+    gc = make_mock_game_connection(
+        state=GameConnectionState.CONNECTED_TO_HOST,
+        player=player
+    )
     game.set_player_option(player.id, "Army", 0)
     game.set_player_option(player.id, "StartSpot", 0)
     game.set_player_option(player.id, "Team", 0)

tests/unit_tests/test_broadcast_service.py

@@ -7,6 +7,14 @@ async def test_broadcast_shutdown(broadcast_service):
     broadcast_service.server.write_broadcast.assert_called_once()
 
 
+async def test_broadcast_ping(broadcast_service):
+    broadcast_service.broadcast_ping()
+
+    broadcast_service.server.write_broadcast.assert_called_once_with(
+        {"command": "ping"}
+    )
+
+
 async def test_wait_report_dirties(broadcast_service):
     await asyncio.wait_for(
         broadcast_service.wait_report_dirtes(),

tests/unit_tests/test_lobbyconnection.py

@@ -585,7 +585,7 @@ async def test_command_admin_closeFA(lobbyconnection, player_factory):
         "user_id": tuna.id
     })
 
-    tuna.lobby_connection.send.assert_any_call({
+    tuna.lobby_connection.write.assert_any_call({
         "command": "notice",
         "style": "kill",
     })