rf: Remove unnecessary global constants from _s3

Author: effigies

templateflow/conf/_s3.py

@@ -25,46 +25,48 @@
 from pathlib import Path
 from tempfile import mkstemp
 
-from templateflow.conf import TF_GET_TIMEOUT, load_data
+from acres import Loader
+
+load_data = Loader(__spec__.parent)
 
 TF_SKEL_URL = (
     'https://raw.githubusercontent.com/templateflow/python-client/'
     '{release}/templateflow/conf/templateflow-skel.{ext}'
 ).format
-TF_SKEL_PATH = load_data('templateflow-skel.zip')
-TF_SKEL_MD5 = load_data.readable('templateflow-skel.md5').read_text()
 
 
-def update(dest, local=True, overwrite=True, silent=False):
+def update(dest, local=True, overwrite=True, silent=False, *, timeout: int):
     """Update an S3-backed TEMPLATEFLOW_HOME repository."""
-    skel_file = Path((_get_skeleton_file() if not local else None) or TF_SKEL_PATH)
+    skel_zip = load_data('templateflow-skel.zip')
+    skel_file = Path((_get_skeleton_file(timeout) if not local else None) or skel_zip)
 
     retval = _update_skeleton(skel_file, dest, overwrite=overwrite, silent=silent)
-    if skel_file != TF_SKEL_PATH:
+    if skel_file != skel_zip:
         skel_file.unlink()
     return retval
 
 
-def _get_skeleton_file():
+def _get_skeleton_file(timeout: int):
     import requests
 
     try:
         r = requests.get(
             TF_SKEL_URL(release='master', ext='md5'),
             allow_redirects=True,
-            timeout=TF_GET_TIMEOUT,
+            timeout=timeout,
         )
     except requests.exceptions.ConnectionError:
         return
 
     if not r.ok:
         return
 
-    if r.content.decode().split()[0] != TF_SKEL_MD5:
+    md5 = load_data.readable('templateflow-skel.md5').read_bytes()
+    if r.content != md5:
         r = requests.get(
             TF_SKEL_URL(release='master', ext='zip'),
             allow_redirects=True,
-            timeout=TF_GET_TIMEOUT,
+            timeout=timeout,
         )
         if r.ok:
             from os import close

templateflow/conf/bids.py

@@ -22,9 +22,10 @@
 #
 """Extending pyBIDS for querying TemplateFlow."""
 
+from acres import Loader
 from bids.layout import BIDSLayout, add_config_paths
 
-from templateflow.conf import load_data
+load_data = Loader(__spec__.parent)
 
 add_config_paths(templateflow=load_data('config.json'))
 

templateflow/tests/test_s3.py

@@ -44,31 +44,36 @@ def test_get_skel_file(tmp_path, monkeypatch):
     # First execution, the S3 stub is created (or datalad install)
     reload(tfc)
 
-    local_md5 = tfc._s3.TF_SKEL_MD5
-    monkeypatch.setattr(tfc._s3, 'TF_SKEL_MD5', 'invent')
-    new_skel = tfc._s3._get_skeleton_file()
+    md5content = b'anything'
+
+    def mock_get(*args, **kwargs):
+        class MockResponse:
+            status_code = 200
+            ok = True
+            content = md5content
+
+        return MockResponse()
+
+    monkeypatch.setattr(requests, 'get', mock_get)
+
+    # Mismatching the local MD5 causes an update
+    new_skel = tfc._s3._get_skeleton_file(timeout=10)
     assert new_skel is not None
     assert Path(new_skel).exists()
-    assert Path(new_skel).stat().st_size > 0
+    assert Path(new_skel).read_bytes() == b'anything'
 
-    latest_md5 = (
-        requests.get(
-            tfc._s3.TF_SKEL_URL(release='master', ext='md5', allow_redirects=True), timeout=10
-        )
-        .content.decode()
-        .split()[0]
-    )
-    monkeypatch.setattr(tfc._s3, 'TF_SKEL_MD5', latest_md5)
-    assert tfc._s3._get_skeleton_file() is None
+    md5content = tfc._s3.load_data.readable('templateflow-skel.md5').read_bytes()
+    # Matching the local MD5 skips the update
+    assert tfc._s3._get_skeleton_file(timeout=10) is None
 
-    monkeypatch.setattr(tfc._s3, 'TF_SKEL_MD5', local_md5)
+    # Bad URL fails to update
     monkeypatch.setattr(tfc._s3, 'TF_SKEL_URL', 'http://weird/{release}/{ext}'.format)
-    assert tfc._s3._get_skeleton_file() is None
+    assert tfc._s3._get_skeleton_file(timeout=10) is None
 
     monkeypatch.setattr(
         tfc._s3, 'TF_SKEL_URL', tfc._s3.TF_SKEL_URL(release='{release}', ext='{ext}z').format
     )
-    assert tfc._s3._get_skeleton_file() is None
+    assert tfc._s3._get_skeleton_file(timeout=10) is None
 
 
 def test_update_s3(tmp_path, monkeypatch):
@@ -78,19 +83,19 @@ def test_update_s3(tmp_path, monkeypatch):
     monkeypatch.setenv('TEMPLATEFLOW_USE_DATALAD', 'off')
     monkeypatch.setenv('TEMPLATEFLOW_HOME', str(newhome))
 
-    assert tfc._s3.update(newhome)
-    assert not tfc._s3.update(newhome, overwrite=False)
+    assert tfc._s3.update(newhome, timeout=10)
+    assert not tfc._s3.update(newhome, overwrite=False, timeout=10)
     for p in (newhome / 'tpl-MNI152NLin6Sym').glob('*.nii.gz'):
         p.unlink()
-    assert tfc._s3.update(newhome, overwrite=False)
+    assert tfc._s3.update(newhome, overwrite=False, timeout=10)
 
     # This should cover the remote zip file fetching
-    monkeypatch.setattr(tfc._s3, 'TF_SKEL_MD5', 'invent')
-    assert tfc._s3.update(newhome, local=False)
-    assert not tfc._s3.update(newhome, local=False, overwrite=False)
+    # monkeypatch.setattr(tfc._s3, 'TF_SKEL_MD5', 'invent')
+    assert tfc._s3.update(newhome, local=False, timeout=10)
+    assert not tfc._s3.update(newhome, local=False, overwrite=False, timeout=10)
     for p in (newhome / 'tpl-MNI152NLin6Sym').glob('*.nii.gz'):
         p.unlink()
-    assert tfc._s3.update(newhome, local=False, overwrite=False)
+    assert tfc._s3.update(newhome, local=False, overwrite=False, timeout=10)
 
 
 def test_s3_400_error(monkeypatch):