fix: Unstable PROMETHEUS_MULTIPROC_DIR
#116
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
- Coverage 94.67% 94.65% -0.02%
==========================================
Files 76 77 +1
Lines 2479 2470 -9
==========================================
- Hits 2347 2338 -9
Misses 132 132 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I don't have context yet, but I'm interested in possibly cleaning up temporary files on startup (just in case the previous run ended unexpectedly), and on shutdown (because it's polite to cleanup resources after use).
Unless there's reason I don't know, the better approach to manage temporary files should be via tempfile as long as we always cleanup.
| def test_main__prometheus_multiproc_remove_dir_on_start_true__expected( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| fs: FakeFilesystem, | ||
| ) -> None: | ||
| # Given | ||
| monkeypatch.delenv("PROMETHEUS_MULTIPROC_DIR") | ||
| monkeypatch.delenv("PROMETHEUS_MULTIPROC_DIR", raising=False) | ||
| monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR_KEEP", "true") | ||
|
|
||
| fs.create_file( | ||
| "/tmp/flagsmith-prometheus/some_metric_file.db", | ||
| create_missing_dirs=True, | ||
| ) | ||
|
|
||
| # When | ||
| main(["flagsmith"]) | ||
|
|
||
| # Then | ||
| assert Path(os.environ["PROMETHEUS_MULTIPROC_DIR"]).exists() | ||
| assert fs.exists("/tmp/flagsmith-prometheus/some_metric_file.db") |
There was a problem hiding this comment.
Is this use case real? i.e. Is there a reason why we need to keep the temporary files generated by this program?
In this PR, we change the following:
PROMETHEUS_MULTIPROC_DIRinstead of relying on a random one offered bytempfile.mkdtempto completely avoid new directories spam reported in temporary directories are not removed flagsmith#5990.