Add logic for cleaning up file handlers; Ignore certain assets for copy

Author: john-b-yang

codeclash/tournaments/pvp.py

@@ -30,10 +30,11 @@ def __init__(
         push: bool = False,
         keep_containers: bool = False,
     ):
+        metadata_file = output_dir / "metadata.json"
+        if metadata_file.exists():
+            raise FileExistsError(f"Metadata file already exists: {metadata_file}")
+
         super().__init__(config, name="PvpTournament", output_dir=output_dir)
-        if self.metadata_file.exists():
-            self.logger.critical(f"Metadata file already exists: {self.metadata_file}")
-            raise FileExistsError(f"Metadata file already exists: {self.metadata_file}")
         self.cleanup_on_end = cleanup
         self.game: CodeArena = get_arena(
             self.config,
@@ -213,3 +214,4 @@ def end(self) -> None:
         """Save output files, clean up game resources and push agents if requested."""
         self._save()
         self.game.end(self.cleanup_on_end)
+        self.cleanup_handlers()

codeclash/tournaments/single_player.py

@@ -141,6 +141,7 @@ def end(self):
         """Clean up game resources."""
         self._save()
         self.game.end(self.cleanup_on_end)
+        self.cleanup_handlers()
 
     def evaluate(self, n_repetitions: int = 3) -> None:
         """Evaluate the agent's performance by

codeclash/tournaments/tournament.py

@@ -1,11 +1,12 @@
+import logging
 import os
 import time
 import traceback
 from pathlib import Path
 
 from codeclash.utils.aws import get_aws_metadata
 from codeclash.utils.environment import create_file_in_container
-from codeclash.utils.log import add_root_file_handler, get_logger
+from codeclash.utils.log import add_root_file_handler, get_logger, remove_file_handler
 
 
 class AbstractTournament:
@@ -22,7 +23,7 @@ def __init__(self, config: dict, *, name: str, output_dir: Path, **kwargs):
             "aws": get_aws_metadata(),
         }
         self.logger = get_logger(self.name, log_path=self.local_output_dir / "tournament.log", emoji="🏆")
-        add_root_file_handler(self.local_output_dir / "everything.log")
+        self._root_file_handler = add_root_file_handler(self.local_output_dir / "everything.log")
 
     @property
     def local_output_dir(self) -> Path:
@@ -33,6 +34,17 @@ def local_output_dir(self) -> Path:
     def get_metadata(self) -> dict:
         return self._metadata
 
+    def cleanup_handlers(self) -> None:
+        """Close and remove file handlers to prevent file descriptor leaks."""
+        # Close the root file handler
+        remove_file_handler(logging.getLogger(), self._root_file_handler)
+
+        # Close logger's file handlers
+        for handler in self.logger.handlers[:]:  # Copy list to avoid modification during iteration
+            if isinstance(handler, logging.FileHandler):
+                self.logger.removeHandler(handler)
+                handler.close()
+
     def _copy_game_log_to_agent(self, agent, round_num: int, log_output: str, dest_path: str = None) -> None:
         """Copy round log to agent environment."""
         try:

codeclash/utils/environment.py

@@ -1,10 +1,14 @@
 import logging
+import shutil
 import subprocess
 import tempfile
 from pathlib import Path
 
 from minisweagent.environments.docker import DockerEnvironment
 
+# Patterns to exclude when copying between containers
+COPY_EXCLUDE_PATTERNS = [".git", "__pycache__"]
+
 
 def assert_zero_exit_code(result: dict, *, logger: logging.Logger | None = None) -> dict:
     if result.get("returncode", 0) != 0:
@@ -49,6 +53,15 @@ def copy_between_containers(
                 f"Failed to copy from {src_container.container_id} to local temp: {result_src.stdout}{result_src.stderr}"
             )
 
+        # Remove excluded patterns
+        for pattern in COPY_EXCLUDE_PATTERNS:
+            excluded_path = temp_path / pattern
+            if excluded_path.exists():
+                if excluded_path.is_dir():
+                    shutil.rmtree(excluded_path)
+                else:
+                    excluded_path.unlink()
+
         # Ensure destination folder exists
         assert_zero_exit_code(dest_container.execute(f"mkdir -p {Path(dest_path).parent}"))
 

codeclash/utils/log.py

@@ -64,8 +64,12 @@ def format(self, record: logging.LogRecord) -> str:
         return capture.get().rstrip()
 
 
-def add_file_handler(logger: logging.Logger, log_path: Path) -> None:
-    """Add a file handler to the logger with standard formatting."""
+def add_file_handler(logger: logging.Logger, log_path: Path) -> logging.FileHandler:
+    """Add a file handler to the logger with standard formatting.
+
+    Returns:
+        The FileHandler that was added (for later cleanup).
+    """
     log_path.parent.mkdir(parents=True, exist_ok=True)
     file_handler = logging.FileHandler(log_path)
     file_handler.setLevel(_FILE_LEVEL)
@@ -78,11 +82,23 @@ def add_file_handler(logger: logging.Logger, log_path: Path) -> None:
     file_handler.setFormatter(file_formatter)
 
     logger.addHandler(file_handler)
+    return file_handler
+
+
+def add_root_file_handler(log_path: Path) -> logging.FileHandler:
+    """Add a file handler to the root logger to capture all log messages.
+
+    Returns:
+        The FileHandler that was added (for later cleanup).
+    """
+    return add_file_handler(logging.getLogger(), log_path)
 
 
-def add_root_file_handler(log_path: Path) -> None:
-    """Add a file handler to the root logger to capture all log messages."""
-    add_file_handler(logging.getLogger(), log_path)
+def remove_file_handler(logger: logging.Logger, handler: logging.FileHandler) -> None:
+    """Remove and close a file handler from the logger."""
+    if handler in logger.handlers:
+        logger.removeHandler(handler)
+    handler.close()
 
 
 def get_logger(name: str, *, emoji: str = "", log_path: Path | None = None) -> logging.Logger: