From dc1c1ebae632a91492d337766942424d95ad3241 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 28 Oct 2025 18:45:24 +0000 Subject: [PATCH 1/8] fix: Temporary directories are not removed --- src/common/core/main.py | 10 +++++++--- tests/integration/core/test_main.py | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/common/core/main.py b/src/common/core/main.py index 07976148..eb7a108e 100644 --- a/src/common/core/main.py +++ b/src/common/core/main.py @@ -8,6 +8,7 @@ from django.core.management import ( execute_from_command_line as django_execute_from_command_line, ) +from environs import Env from common.core.cli import healthcheck @@ -30,6 +31,7 @@ def ensure_cli_env() -> typing.Generator[None, None, None]: main() ``` """ + env = Env() ctx = contextlib.ExitStack() # TODO @khvn26 Move logging setup to here @@ -43,9 +45,11 @@ def ensure_cli_env() -> typing.Generator[None, None, None]: os.environ.setdefault("DJANGO_SETTINGS_MODULE", "settings.dev") # Set up Prometheus' multiprocess mode - if "PROMETHEUS_MULTIPROC_DIR" not in os.environ: - prometheus_multiproc_dir_name = tempfile.mkdtemp() - + if not env.str("PROMETHEUS_MULTIPROC_DIR", ""): + delete = not env.bool("PROMETHEUS_MULTIPROC_DIR_KEEP", False) + prometheus_multiproc_dir_name = ctx.enter_context( + tempfile.TemporaryDirectory(delete=delete) + ) logger.info( "Created %s for Prometheus multi-process mode", prometheus_multiproc_dir_name, diff --git a/tests/integration/core/test_main.py b/tests/integration/core/test_main.py index 7287b4c3..74019ba7 100644 --- a/tests/integration/core/test_main.py +++ b/tests/integration/core/test_main.py @@ -1,6 +1,10 @@ +import os +from pathlib import Path + import django import pytest from django.core.management import ManagementUtility +from pyfakefs.fake_filesystem import FakeFilesystem from pytest_httpserver import HTTPServer from common.core.main import main @@ -104,3 +108,24 @@ def test_main__healthcheck_http__server_invalid_response__runs_expected( # When & Then with pytest.raises(Exception): main(argv) + + +def test_main__prometheus_multiproc_remove_dir_on_exit_default__expected() -> None: + # Given & When + main(["flagsmith"]) + + # Then + assert not Path(os.environ["PROMETHEUS_MULTIPROC_DIR"]).exists() + + +def test_main__prometheus_multiproc_remove_dir_on_exit_true__expected( + fs: FakeFilesystem, +) -> None: + # Given + os.environ["PROMETHEUS_MULTIPROC_DIR_KEEP"] = "true" + + # When + main(["flagsmith"]) + + # Then + assert Path(os.environ["PROMETHEUS_MULTIPROC_DIR"]).exists() From 002c4870b7c0d15eb39847a3db2d780eceb2031c Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 30 Oct 2025 11:51:59 +0000 Subject: [PATCH 2/8] add shim for 3.11 --- src/common/core/main.py | 4 +-- src/common/core/utils.py | 55 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/common/core/main.py b/src/common/core/main.py index eb7a108e..d1b7bcdb 100644 --- a/src/common/core/main.py +++ b/src/common/core/main.py @@ -2,7 +2,6 @@ import logging import os import sys -import tempfile import typing from django.core.management import ( @@ -11,6 +10,7 @@ from environs import Env from common.core.cli import healthcheck +from common.core.utils import TemporaryDirectory logger = logging.getLogger(__name__) @@ -48,7 +48,7 @@ def ensure_cli_env() -> typing.Generator[None, None, None]: if not env.str("PROMETHEUS_MULTIPROC_DIR", ""): delete = not env.bool("PROMETHEUS_MULTIPROC_DIR_KEEP", False) prometheus_multiproc_dir_name = ctx.enter_context( - tempfile.TemporaryDirectory(delete=delete) + TemporaryDirectory(delete=delete) ) logger.info( "Created %s for Prometheus multi-process mode", diff --git a/src/common/core/utils.py b/src/common/core/utils.py index 301edcd6..82fc08c5 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -2,9 +2,18 @@ import logging import pathlib import random +import sys +import tempfile from functools import lru_cache from itertools import cycle -from typing import Iterator, Literal, NotRequired, TypedDict, TypeVar, get_args +from typing import ( + Iterator, + Literal, + NotRequired, + TypedDict, + TypeVar, + get_args, +) from django.conf import settings from django.contrib.auth import get_user_model @@ -186,3 +195,47 @@ def using_database_replica( return manager return manager.db_manager(chosen_replica) + + +if sys.version_info >= (3, 12): + # Already has the desired behavior; re-export for uniform imports. + TemporaryDirectory = tempfile.TemporaryDirectory +else: + + class TemporaryDirectory(tempfile.TemporaryDirectory): + """ + Python 3.11-compatible TemporaryDirectory with a 3.12-style 'delete' flag. + + Args: + suffix (str | None): directory name suffix + prefix (str | None): directory name prefix + dir (str | bytes | os.PathLike | None): parent directory + ignore_cleanup_errors (bool): best-effort cleanup (same as 3.11) + delete (bool): if False, disable automatic cleanup on context-exit/GC. + Explicit .cleanup() still deletes. + """ + + def __init__( + self, + *args: object, + delete: bool = True, + ): + self.delete = delete + super().__init__(*args) + + def __exit__(self, exc_type, exc, tb): + """On context exit, only cleanup if delete=True.""" + if self.delete: + super().__exit__(exc_type, exc, tb) + # Return False to propagate exceptions like the stdlib does. + return False + + def _cleanup(self, name, warn_message): + """ + Called by the weakref finalizer on GC in 3.11. + Respect delete=False by doing nothing in that case. + """ + if not self.delete: + return + # Defer to stdlib implementation for actual removal. + return super()._cleanup(name, warn_message) From 6f765d66f83c68a5ecf9c53488dd9547a7c8e00f Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 30 Oct 2025 12:24:27 +0000 Subject: [PATCH 3/8] fix typing errors --- src/common/core/utils.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/common/core/utils.py b/src/common/core/utils.py index 82fc08c5..52f81dac 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -201,6 +201,8 @@ def using_database_replica( # Already has the desired behavior; re-export for uniform imports. TemporaryDirectory = tempfile.TemporaryDirectory else: + from os import PathLike + from types import TracebackType class TemporaryDirectory(tempfile.TemporaryDirectory): """ @@ -217,20 +219,34 @@ class TemporaryDirectory(tempfile.TemporaryDirectory): def __init__( self, - *args: object, + suffix: str | None = None, + prefix: str | None = None, + dir: str | PathLike[str] | None = None, + ignore_cleanup_errors: bool = False, + *, delete: bool = True, - ): + ) -> None: self.delete = delete - super().__init__(*args) - - def __exit__(self, exc_type, exc, tb): + super().__init__( + suffix=suffix, + prefix=prefix, + dir=dir, + ignore_cleanup_errors=ignore_cleanup_errors, + ) + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc: BaseException | None, + tb: TracebackType | None, + ) -> bool: """On context exit, only cleanup if delete=True.""" if self.delete: super().__exit__(exc_type, exc, tb) # Return False to propagate exceptions like the stdlib does. return False - def _cleanup(self, name, warn_message): + def _cleanup(self, name: str, warn_message: str) -> None: """ Called by the weakref finalizer on GC in 3.11. Respect delete=False by doing nothing in that case. From 071c6e3cb321bac8d0547667d1ef862c3bcf1928 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 30 Oct 2025 14:39:26 +0000 Subject: [PATCH 4/8] typing --- src/common/core/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/core/utils.py b/src/common/core/utils.py index 52f81dac..be5baaa3 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -203,8 +203,9 @@ def using_database_replica( else: from os import PathLike from types import TracebackType + from typing import Literal - class TemporaryDirectory(tempfile.TemporaryDirectory): + class TemporaryDirectory(tempfile.TemporaryDirectory[str]): """ Python 3.11-compatible TemporaryDirectory with a 3.12-style 'delete' flag. @@ -239,7 +240,7 @@ def __exit__( exc_type: type[BaseException] | None, exc: BaseException | None, tb: TracebackType | None, - ) -> bool: + ) -> Literal[False]: """On context exit, only cleanup if delete=True.""" if self.delete: super().__exit__(exc_type, exc, tb) @@ -254,4 +255,4 @@ def _cleanup(self, name: str, warn_message: str) -> None: if not self.delete: return # Defer to stdlib implementation for actual removal. - return super()._cleanup(name, warn_message) + return tempfile.TemporaryDirectory._cleanup(type(self), name, warn_message) From 50dd69186f31a101555c618f06297f7e4973f9ca Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 30 Oct 2025 14:51:17 +0000 Subject: [PATCH 5/8] fix typing errors --- src/common/core/utils.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/common/core/utils.py b/src/common/core/utils.py index be5baaa3..711f694f 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -240,12 +240,10 @@ def __exit__( exc_type: type[BaseException] | None, exc: BaseException | None, tb: TracebackType | None, - ) -> Literal[False]: + ) -> None: """On context exit, only cleanup if delete=True.""" if self.delete: super().__exit__(exc_type, exc, tb) - # Return False to propagate exceptions like the stdlib does. - return False def _cleanup(self, name: str, warn_message: str) -> None: """ @@ -255,4 +253,4 @@ def _cleanup(self, name: str, warn_message: str) -> None: if not self.delete: return # Defer to stdlib implementation for actual removal. - return tempfile.TemporaryDirectory._cleanup(type(self), name, warn_message) + return type(self)._cleanup(self, name, warn_message) From f03dcd5ebf9a077cda8d1bf2c39fde04f897e4fb Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 30 Oct 2025 15:35:34 +0000 Subject: [PATCH 6/8] fix everything --- src/common/core/utils.py | 71 ++++++++--------------------- tests/integration/core/test_main.py | 6 ++- 2 files changed, 23 insertions(+), 54 deletions(-) diff --git a/src/common/core/utils.py b/src/common/core/utils.py index 711f694f..3b68908b 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -201,56 +201,21 @@ def using_database_replica( # Already has the desired behavior; re-export for uniform imports. TemporaryDirectory = tempfile.TemporaryDirectory else: - from os import PathLike - from types import TracebackType - from typing import Literal - - class TemporaryDirectory(tempfile.TemporaryDirectory[str]): - """ - Python 3.11-compatible TemporaryDirectory with a 3.12-style 'delete' flag. - - Args: - suffix (str | None): directory name suffix - prefix (str | None): directory name prefix - dir (str | bytes | os.PathLike | None): parent directory - ignore_cleanup_errors (bool): best-effort cleanup (same as 3.11) - delete (bool): if False, disable automatic cleanup on context-exit/GC. - Explicit .cleanup() still deletes. - """ - - def __init__( - self, - suffix: str | None = None, - prefix: str | None = None, - dir: str | PathLike[str] | None = None, - ignore_cleanup_errors: bool = False, - *, - delete: bool = True, - ) -> None: - self.delete = delete - super().__init__( - suffix=suffix, - prefix=prefix, - dir=dir, - ignore_cleanup_errors=ignore_cleanup_errors, - ) - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc: BaseException | None, - tb: TracebackType | None, - ) -> None: - """On context exit, only cleanup if delete=True.""" - if self.delete: - super().__exit__(exc_type, exc, tb) - - def _cleanup(self, name: str, warn_message: str) -> None: - """ - Called by the weakref finalizer on GC in 3.11. - Respect delete=False by doing nothing in that case. - """ - if not self.delete: - return - # Defer to stdlib implementation for actual removal. - return type(self)._cleanup(self, name, warn_message) + import contextlib + import tempfile + from typing import ContextManager, Generator + + def TemporaryDirectory( + suffix: str | None = None, + prefix: str | None = None, + dir: str | None = None, + delete: bool = True, + ) -> ContextManager[str]: + if delete: + return tempfile.TemporaryDirectory(suffix, prefix, dir) + + @contextlib.contextmanager + def _tmpdir() -> Generator[str, None, None]: + yield tempfile.mkdtemp(suffix, prefix, dir) + + return _tmpdir() diff --git a/tests/integration/core/test_main.py b/tests/integration/core/test_main.py index 74019ba7..b302be2a 100644 --- a/tests/integration/core/test_main.py +++ b/tests/integration/core/test_main.py @@ -111,7 +111,10 @@ def test_main__healthcheck_http__server_invalid_response__runs_expected( def test_main__prometheus_multiproc_remove_dir_on_exit_default__expected() -> None: - # Given & When + # Given + os.environ.pop("PROMETHEUS_MULTIPROC_DIR_KEEP", None) + + # When main(["flagsmith"]) # Then @@ -122,6 +125,7 @@ def test_main__prometheus_multiproc_remove_dir_on_exit_true__expected( fs: FakeFilesystem, ) -> None: # Given + os.environ.pop("PROMETHEUS_MULTIPROC_DIR", None) os.environ["PROMETHEUS_MULTIPROC_DIR_KEEP"] = "true" # When From 71d3bdf8acf7d3f8190b71aa5c0f971bf58d3ac3 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 30 Oct 2025 16:33:48 +0000 Subject: [PATCH 7/8] remove extra import --- src/common/core/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/common/core/utils.py b/src/common/core/utils.py index 3b68908b..a9ffca5c 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -202,7 +202,6 @@ def using_database_replica( TemporaryDirectory = tempfile.TemporaryDirectory else: import contextlib - import tempfile from typing import ContextManager, Generator def TemporaryDirectory( From c10705f049410f7c5bf92933c86756704ee593c8 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 3 Nov 2025 11:24:06 +0000 Subject: [PATCH 8/8] add docs --- src/common/core/utils.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/common/core/utils.py b/src/common/core/utils.py index a9ffca5c..861f2b94 100644 --- a/src/common/core/utils.py +++ b/src/common/core/utils.py @@ -208,8 +208,22 @@ def TemporaryDirectory( suffix: str | None = None, prefix: str | None = None, dir: str | None = None, + *, delete: bool = True, ) -> ContextManager[str]: + """ + Create a temporary directory with optional cleanup control. + + This wrapper exists because Python 3.12 changed TemporaryDirectory's behavior + by adding a 'delete' parameter, which doesn't exist in Python 3.11. This + function provides a consistent API across both versions. + + When delete=True, uses the stdlib's TemporaryDirectory (auto-cleanup). + When delete=False, creates a directory with mkdtemp that persists after + the context manager exits, matching Python 3.12's delete=False behavior. + + See https://docs.python.org/3.12/library/tempfile.html#tempfile.TemporaryDirectory for usage details. + """ if delete: return tempfile.TemporaryDirectory(suffix, prefix, dir)