Add env var for version cache timeout

As part of the Newton midcycle, it was discussed that it might
be nice to be able to specify the version cache timeout value
via an environment variable, rather than the current hard-coded
value.

Fixes-bug: #1596734
Change-Id: I2bc77c64b764f6c22574a30b0e5af4ca6feff29f

ironicclient/common/filecache.py

@@ -14,18 +14,23 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import logging
 import os
 
 import appdirs
 import dogpile.cache
 
+from ironicclient.common.i18n import _LW
+
+LOG = logging.getLogger(__name__)
 
 AUTHOR = 'openstack'
 PROGNAME = 'python-ironicclient'
 
-CACHE_DIR = appdirs.user_cache_dir(PROGNAME, AUTHOR)
-CACHE_FILENAME = os.path.join(CACHE_DIR, 'ironic-api-version.dbm')
 CACHE = None
+CACHE_DIR = appdirs.user_cache_dir(PROGNAME, AUTHOR)
+CACHE_EXPIRY_ENV_VAR = 'IRONICCLIENT_CACHE_EXPIRY'  # environment variable
+CACHE_FILENAME = os.path.join(CACHE_DIR, 'ironic-api-version.dbm')
 DEFAULT_EXPIRY = 300  # seconds
 
 
@@ -38,9 +43,22 @@ def _get_cache():
         if not os.path.exists(CACHE_DIR):
             os.makedirs(CACHE_DIR)
 
+        # Use the cache expiry if specified in an env var
+        expiry_time = os.environ.get(CACHE_EXPIRY_ENV_VAR, DEFAULT_EXPIRY)
+        try:
+            expiry_time = int(expiry_time)
+        except ValueError:
+            LOG.warning(_LW("Environment variable %(env_var)s should be an "
+                            "integer (not '%(curr_val)s') using default "
+                            "timeout of %(default)s instead"),
+                        {'env_var': CACHE_EXPIRY_ENV_VAR,
+                         'curr_val': expiry_time,
+                         'default': DEFAULT_EXPIRY})
+            expiry_time = DEFAULT_EXPIRY
+
         CACHE = dogpile.cache.make_region(key_mangler=str).configure(
             'dogpile.cache.dbm',
-            expiration_time=DEFAULT_EXPIRY,
+            expiration_time=expiry_time,
             arguments={
                 "filename": CACHE_FILENAME,
             }

ironicclient/tests/unit/common/test_filecache.py

@@ -29,12 +29,15 @@ class FileCacheTest(utils.BaseTestCase):
         result = filecache._build_key(None, None)
         self.assertEqual('None:None', result)
 
+    @mock.patch.object(os.environ, 'get', autospec=True)
     @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'makedirs', autospec=True)
     @mock.patch.object(dogpile.cache, 'make_region', autospec=True)
     def test__get_cache_mkdir(self, mock_makeregion, mock_makedirs,
-                              mock_exists):
+                              mock_exists, mock_get):
         cache_val = 6
+        # If not present in the env, get will return the defaulted value
+        mock_get.return_value = filecache.DEFAULT_EXPIRY
         filecache.CACHE = None
         mock_exists.return_value = False
         cache_region = mock.Mock(spec=dogpile.cache.region.CacheRegion)
@@ -43,6 +46,62 @@ class FileCacheTest(utils.BaseTestCase):
         self.assertEqual(cache_val, filecache._get_cache())
         mock_exists.assert_called_once_with(filecache.CACHE_DIR)
         mock_makedirs.assert_called_once_with(filecache.CACHE_DIR)
+        mock_get.assert_called_once_with(filecache.CACHE_EXPIRY_ENV_VAR,
+                                         mock.ANY)
+        cache_region.configure.assert_called_once_with(
+            mock.ANY,
+            arguments=mock.ANY,
+            expiration_time=filecache.DEFAULT_EXPIRY)
+
+    @mock.patch.object(os.environ, 'get', autospec=True)
+    @mock.patch.object(os.path, 'exists', autospec=True)
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(dogpile.cache, 'make_region', autospec=True)
+    def test__get_cache_expiry_set(self, mock_makeregion, mock_makedirs,
+                                   mock_exists, mock_get):
+        cache_val = 5643
+        cache_expiry = '78'
+        mock_get.return_value = cache_expiry
+        filecache.CACHE = None
+        mock_exists.return_value = False
+        cache_region = mock.Mock(spec=dogpile.cache.region.CacheRegion)
+        cache_region.configure.return_value = cache_val
+        mock_makeregion.return_value = cache_region
+        self.assertEqual(cache_val, filecache._get_cache())
+        mock_get.assert_called_once_with(filecache.CACHE_EXPIRY_ENV_VAR,
+                                         mock.ANY)
+        cache_region.configure.assert_called_once_with(
+            mock.ANY,
+            arguments=mock.ANY,
+            expiration_time=int(cache_expiry))
+
+    @mock.patch.object(filecache.LOG, 'warning', autospec=True)
+    @mock.patch.object(os.environ, 'get', autospec=True)
+    @mock.patch.object(os.path, 'exists', autospec=True)
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(dogpile.cache, 'make_region', autospec=True)
+    def test__get_cache_expiry_set_invalid(self, mock_makeregion,
+                                           mock_makedirs, mock_exists,
+                                           mock_get, mock_log):
+        cache_val = 5643
+        cache_expiry = 'Rollenhagen'
+        mock_get.return_value = cache_expiry
+        filecache.CACHE = None
+        mock_exists.return_value = False
+        cache_region = mock.Mock(spec=dogpile.cache.region.CacheRegion)
+        cache_region.configure.return_value = cache_val
+        mock_makeregion.return_value = cache_region
+        self.assertEqual(cache_val, filecache._get_cache())
+        mock_get.assert_called_once_with(filecache.CACHE_EXPIRY_ENV_VAR,
+                                         mock.ANY)
+        cache_region.configure.assert_called_once_with(
+            mock.ANY,
+            arguments=mock.ANY,
+            expiration_time=filecache.DEFAULT_EXPIRY)
+        log_dict = {'curr_val': cache_expiry,
+                    'default': filecache.DEFAULT_EXPIRY,
+                    'env_var': filecache.CACHE_EXPIRY_ENV_VAR}
+        mock_log.assert_called_once_with(mock.ANY, log_dict)
 
     @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'makedirs', autospec=True)

releasenotes/notes/add-environment-variable-to-specify-version-cache-timeout-dfa5f6d4af0ea1d3.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Add support for specifying the version cache timeout via an environment
+    variable (IRONICCLIENT_CACHE_EXPIRY).