21020: Make arvados.api.http_cache get a path from the environment
authorBrett Smith <brett.smith@curii.com>
Mon, 20 May 2024 21:12:33 +0000 (17:12 -0400)
committerBrett Smith <brett.smith@curii.com>
Fri, 24 May 2024 21:02:45 +0000 (17:02 -0400)
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith@curii.com>

sdk/python/arvados/api.py
sdk/python/tests/test_cache.py

index 8a17e42fcb3af881e517d8d580e3b5bdb4c25e41..6b6ee36b3ecb7ed803ac4cb6075254ba2439e074 100644 (file)
@@ -159,25 +159,22 @@ def http_cache(data_type: str) -> cache.SafeHTTPCache:
     """Set up an HTTP file cache
 
     This function constructs and returns an `arvados.cache.SafeHTTPCache`
-    backed by the filesystem under `~/.cache/arvados/`, or `None` if the
-    directory cannot be set up. The return value can be passed to
+    backed by the filesystem under a cache directory from the environment, or
+    `None` if the directory cannot be set up. The return value can be passed to
     `httplib2.Http` as the `cache` argument.
 
     Arguments:
 
-    * data_type: str --- The name of the subdirectory under `~/.cache/arvados`
+    * data_type: str --- The name of the subdirectory
       where data is cached.
     """
     try:
-        homedir = pathlib.Path.home()
-    except RuntimeError:
+        path = util._BaseDirectories('CACHE').storage_path() / data_type
+        path.mkdir(exist_ok=True)
+    except (OSError, RuntimeError):
         return None
-    path = pathlib.Path(homedir, '.cache', 'arvados', data_type)
-    try:
-        path.mkdir(parents=True, exist_ok=True)
-    except OSError:
-        return None
-    return cache.SafeHTTPCache(str(path), max_age=60*60*24*2)
+    else:
+        return cache.SafeHTTPCache(str(path), max_age=60*60*24*2)
 
 def api_client(
         version: str,
@@ -211,8 +208,7 @@ def api_client(
     Keyword-only arguments:
 
     * cache: bool --- If true, loads the API discovery document from, or
-      saves it to, a cache on disk (located at
-      `~/.cache/arvados/discovery`).
+      saves it to, a cache on disk.
 
     * http: httplib2.Http | None --- The HTTP client object the API client
       object will use to make requests.  If not provided, this function will
index 41984a5bf916dd7767855e0ca9f6b58912fd6c76..33a19cbdbc3520b4d8deac3796f2dfdb53ca4824 100644 (file)
@@ -11,10 +11,12 @@ import tempfile
 import threading
 import unittest
 
+import pytest
 from unittest import mock
 
 import arvados
 import arvados.cache
+import arvados.util
 from . import run_test_server
 
 def _random(n):
@@ -45,6 +47,23 @@ class CacheTestThread(threading.Thread):
                 raise
 
 
+class TestAPIHTTPCache:
+    @pytest.mark.parametrize('data_type', ['discovery', 'keep'])
+    def test_good_storage(self, tmp_path, monkeypatch, data_type):
+        monkeypatch.setattr(arvados.util._BaseDirectories, 'storage_path', lambda _: tmp_path)
+        actual = arvados.http_cache(data_type)
+        assert actual is not None
+        assert (tmp_path / data_type).is_dir()
+
+    @pytest.mark.parametrize('error', [RuntimeError, FileExistsError, PermissionError])
+    def test_unwritable_storage(self, monkeypatch, error):
+        def fail(self):
+            raise error()
+        monkeypatch.setattr(arvados.util._BaseDirectories, 'storage_path', fail)
+        actual = arvados.http_cache('unwritable')
+        assert actual is None
+
+
 class CacheTest(unittest.TestCase):
     def setUp(self):
         self._dir = tempfile.mkdtemp()
@@ -52,17 +71,6 @@ class CacheTest(unittest.TestCase):
     def tearDown(self):
         shutil.rmtree(self._dir)
 
-    def test_cache_create_error(self):
-        _, filename = tempfile.mkstemp()
-        home_was = os.environ['HOME']
-        os.environ['HOME'] = filename
-        try:
-            c = arvados.http_cache('test')
-            self.assertEqual(None, c)
-        finally:
-            os.environ['HOME'] = home_was
-            os.unlink(filename)
-
     def test_cache_crud(self):
         c = arvados.cache.SafeHTTPCache(self._dir, max_age=0)
         url = 'https://example.com/foo?bar=baz'