fixes for long-broken cache fallback when cache dir not findable

eteq · eteq · commit 8f20d4dd4ad2 · 2012-11-16T11:21:24.000-05:00
diff --git a/astropy/utils/data.py b/astropy/utils/data.py
@@ -753,10 +753,9 @@ def download_file(remote_url, cache=False):
             cache = False
             missing_cache = True  # indicates that the cache is missing to raise a warning later
 
-        _acquire_download_cache_lock()
-
+        if not missing_cache:
+            _acquire_download_cache_lock()
     try:
-
         if cache:
             with _open_shelve(urlmapfn, True) as url2hash:
                 if str(remote_url) in url2hash:
@@ -789,17 +788,15 @@ def download_file(remote_url, cache=False):
                         block = remote.read(DOWNLOAD_CACHE_BLOCK_SIZE())
 
         if cache:
-
             with _open_shelve(urlmapfn, True) as url2hash:
                 local_path = os.path.join(dldir, hash.hexdigest())
                 move(f.name, local_path)
                 url2hash[str(remote_url)] = local_path
-
         else:
-
             local_path = f.name
             if missing_cache:
-                msg = 'File downloaded to temp file due to lack of cache access.'
+                msg = ('File downloaded to temporary location due to problem '
+                       'with cache directory and will not be cached.')
                 warn(CacheMissingWarning(msg, local_path))
             if DELETE_TEMPORARY_DOWNLOADS_AT_EXIT():
                 global _tempfilestodel
@@ -810,7 +807,7 @@ def download_file(remote_url, cache=False):
             e.reason.args = (e.reason.errno, e.reason.strerror)
         raise e
     finally:
-        if cache:
+        if cache and not missing_cache:
             _release_download_cache_lock()
 
     return local_path
diff --git a/astropy/utils/tests/test_data.py b/astropy/utils/tests/test_data.py
@@ -137,19 +137,24 @@ def test_get_pkg_data_contents():
 @remote_data
 def test_data_noastropy_fallback(monkeypatch, recwarn):
     """
-    Tests to make sure configuration items fall back to their defaults when
-    there's a problem accessing the astropy directory
+    Tests to make sure the default behavior when the cache directory can't
+    be located is correct
     """
     from pytest import raises
     from .. import data
     from ...config import paths
 
+    # needed for testing the *real* lock at the end
+    lockdir = os.path.join(_get_download_cache_locs()[0], 'lock')
+
     #better yet, set the configuration to make sure the temp files are deleted
     data.DELETE_TEMPORARY_DOWNLOADS_AT_EXIT.set(True)
 
-    #make sure the config directory is not searched
+    #make sure the config and cache directories are not searched
     monkeypatch.setenv('XDG_CONFIG_HOME', 'foo')
     monkeypatch.delenv('XDG_CONFIG_HOME')
+    monkeypatch.setenv('XDG_CACHE_HOME', 'bar')
+    monkeypatch.delenv('XDG_CACHE_HOME')
 
     # make sure the _find_or_create_astropy_dir function fails as though the
     # astropy dir could not be accessed
@@ -162,7 +167,7 @@ def osraiser(dirnm, linkto):
         paths.get_cache_dir()
 
     #first try with cache
-    fnout = data.get_pkg_data_filename(TESTURL)
+    fnout = data.download_file(TESTURL, cache=True)
     assert os.path.isfile(fnout)
 
     assert len(recwarn.list) > 1
@@ -172,7 +177,7 @@ def osraiser(dirnm, linkto):
     assert w1.category == data.CacheMissingWarning
     assert 'Remote data cache could not be accessed' in w1.message.args[0]
     assert w2.category == data.CacheMissingWarning
-    assert 'File downloaded to temp file' in w2.message.args[0]
+    assert 'File downloaded to temporary location' in w2.message.args[0]
     assert fnout == w2.message.args[1]
 
     #clearing the cache should be a no-up that doesn't affect fnout
@@ -192,13 +197,14 @@ def osraiser(dirnm, linkto):
     assert 'Not clearing data cache - cache inacessable' in str(w3.message)
 
     #now try with no cache
-    with data.get_pkg_data_fileobj(TESTURL, cache=False) as googlepage:
+    fnnocache = data.download_file(TESTURL, cache=False)
+    with open(fnnocache) as googlepage:
         assert googlepage.read().decode().find('oogle</title>') > -1
 
     #no warnings should be raise in fileobj because cache is unnecessary
     assert len(recwarn.list) == 0
 
-    lockdir = os.path.join(_get_download_cache_locs()[0], 'lock')
+    # lockdir determined above as the *real* lockdir, not the temp one
     assert not os.path.isdir(lockdir), 'Cache dir lock was not released!'
 
 
