From 950c2337e22388544df30b956e282dc1d44c2f0f Mon Sep 17 00:00:00 2001 From: Dhananjay Balan Date: Wed, 25 Oct 2017 16:06:29 +0200 Subject: [PATCH 1/3] Adds a helpful error message when multiprocessing sync dir env is not set. On head it fails with: ERROR in app: Exception on /metrics [GET] AttributeError: 'NoneType' object has no attribute 'endswith' With this change: File "build/bdist.linux-x86_64/egg/prometheus_client/multiprocess.py", line 16, in __init__ raise ValueError('env prometheus_multiproc_dir is not set or not accessible') ValueError: env prometheus_multiproc_dir is not set or not accessible --- prometheus_client/multiprocess.py | 2 ++ tests/test_multiprocess.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/prometheus_client/multiprocess.py b/prometheus_client/multiprocess.py index 2fa97168..4c490f6c 100644 --- a/prometheus_client/multiprocess.py +++ b/prometheus_client/multiprocess.py @@ -12,6 +12,8 @@ class MultiProcessCollector(object): """Collector for files for multi-process mode.""" def __init__(self, registry, path=os.environ.get('prometheus_multiproc_dir')): + if not path or not os.path.exists(path): + raise ValueError('env prometheus_multiproc_dir is not set or not accessible') self._path = path if registry: registry.register(self) diff --git a/tests/test_multiprocess.py b/tests/test_multiprocess.py index d122dbbf..2d8673f2 100644 --- a/tests/test_multiprocess.py +++ b/tests/test_multiprocess.py @@ -158,3 +158,9 @@ def test_multi_expansion(self): def tearDown(self): os.unlink(self.tempfile) + +class TestUnsetEnv(unittest.TestCase): + def test_unset_syncdir_env(self): + self.registry = CollectorRegistry() + with self.assertRaises(ValueError): + MultiProcessCollector(self.registry) From 2f0479a47471da133aa6873b240e3d4dab082f93 Mon Sep 17 00:00:00 2001 From: Dhananjay Balan Date: Thu, 26 Oct 2017 12:37:24 +0200 Subject: [PATCH 2/3] Using assertRaises with a context manager is not supported in py2.6 --- tests/test_multiprocess.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_multiprocess.py b/tests/test_multiprocess.py index 2d8673f2..e6213c7c 100644 --- a/tests/test_multiprocess.py +++ b/tests/test_multiprocess.py @@ -162,5 +162,8 @@ def tearDown(self): class TestUnsetEnv(unittest.TestCase): def test_unset_syncdir_env(self): self.registry = CollectorRegistry() - with self.assertRaises(ValueError): - MultiProcessCollector(self.registry) + self.assertRaises( + ValueError, + MultiProcessCollector, + self.registry + ) From 8c93850490a12649661768329cb761b85bfa19f0 Mon Sep 17 00:00:00 2001 From: Dhananjay Balan Date: Thu, 26 Oct 2017 12:53:50 +0200 Subject: [PATCH 3/3] Should fail if the multiproc path is not a directory. --- prometheus_client/multiprocess.py | 4 ++-- tests/test_multiprocess.py | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/prometheus_client/multiprocess.py b/prometheus_client/multiprocess.py index 4c490f6c..a45d1ec7 100644 --- a/prometheus_client/multiprocess.py +++ b/prometheus_client/multiprocess.py @@ -12,8 +12,8 @@ class MultiProcessCollector(object): """Collector for files for multi-process mode.""" def __init__(self, registry, path=os.environ.get('prometheus_multiproc_dir')): - if not path or not os.path.exists(path): - raise ValueError('env prometheus_multiproc_dir is not set or not accessible') + if not path or not os.path.isdir(path): + raise ValueError('env prometheus_multiproc_dir is not set or not a directory') self._path = path if registry: registry.register(self) diff --git a/tests/test_multiprocess.py b/tests/test_multiprocess.py index e6213c7c..8f282d18 100644 --- a/tests/test_multiprocess.py +++ b/tests/test_multiprocess.py @@ -160,10 +160,26 @@ def tearDown(self): os.unlink(self.tempfile) class TestUnsetEnv(unittest.TestCase): - def test_unset_syncdir_env(self): + def setUp(self): self.registry = CollectorRegistry() + fp, self.tmpfl = tempfile.mkstemp() + os.close(fp) + + def test_unset_syncdir_env(self): self.assertRaises( ValueError, MultiProcessCollector, self.registry ) + + def test_file_syncpath(self): + registry = CollectorRegistry() + self.assertRaises( + ValueError, + MultiProcessCollector, + registry, + self.tmpfl + ) + + def tearDown(self): + os.remove(self.tmpfl)