Merge pull request #523 from dbic/bf-retry-load_json

BF(workaround): retry loading json multiple times

Author: yarikoptic

heudiconv/bids.py

@@ -157,7 +157,7 @@ def populate_aggregated_jsons(path):
         # TODO: if we are to fix it, then old ones (without _acq) should be
         # removed first
         task = re.sub('.*_(task-[^_\.]*(_acq-[^_\.]*)?)_.*', r'\1', fpath)
-        json_ = load_json(fpath)
+        json_ = load_json(fpath, retry=100)
         if task not in tasks:
             tasks[task] = json_
         else:
@@ -212,7 +212,7 @@ def populate_aggregated_jsons(path):
             "CogAtlasID": "http://www.cognitiveatlas.org/task/id/TODO",
         }
         if op.lexists(task_file):
-            j = load_json(task_file)
+            j = load_json(task_file, retry=100)
             # Retain possibly modified placeholder fields
             for f in placeholders:
                 if f in j:

heudiconv/tests/test_utils.py

@@ -2,6 +2,8 @@
 import os
 import os.path as op
 
+import mock
+
 from heudiconv.utils import (
     get_known_heuristics_with_descriptions,
     get_heuristic_description,
@@ -77,6 +79,13 @@ def test_load_json(tmpdir, caplog):
     with pytest.raises(JSONDecodeError):
         load_json(str(invalid_json_file))
 
+    # and even if we ask to retry a few times -- should be the same
+    with pytest.raises(JSONDecodeError):
+        load_json(str(invalid_json_file), retry=3)
+
+    with pytest.raises(FileNotFoundError):
+        load_json("absent123not.there", retry=3)
+
     assert ifname in caplog.text
 
     # test valid json
@@ -87,6 +96,23 @@ def test_load_json(tmpdir, caplog):
 
     assert load_json(valid_json_file) == vcontent
 
+    calls = [0]
+    json_load = json.load
+
+    def json_load_patched(fp):
+        calls[0] += 1
+        if calls[0] == 1:
+            # just reuse bad file
+            load_json(str(invalid_json_file))
+        elif calls[0] == 2:
+            raise FileNotFoundError()
+        else:
+            return json_load(fp)
+
+    with mock.patch.object(json, 'load', json_load_patched):
+        assert load_json(valid_json_file, retry=3) == vcontent
+
+
 
 def test_get_datetime():
     """

heudiconv/utils.py

@@ -14,6 +14,7 @@
 from glob import glob
 from subprocess import check_output
 from datetime import datetime
+from time import sleep
 
 from nipype.utils.filemanip import which
 
@@ -147,24 +148,40 @@ def write_config(outfile, info):
         fp.writelines(PrettyPrinter().pformat(info))
 
 
-def load_json(filename):
+def load_json(filename, retry=0):
     """Load data from a json file
 
     Parameters
     ----------
     filename : str
         Filename to load data from.
+    retry: int, optional
+        Number of times to retry opening/loading the file in case of
+        failure.  Code will sleep for 0.1 seconds between retries.
+        Could be used in code which is not sensitive to order effects
+        (e.g. like populating bids templates where the last one to
+        do it, would make sure it would be the correct/final state).
 
     Returns
     -------
     data : dict
     """
-    try:
-        with open(filename, 'r') as fp:
-            data = json.load(fp)
-    except JSONDecodeError:
-        lgr.error("{fname} is not a valid json file".format(fname=filename))
-        raise
+    assert retry >= 0
+    for i in range(retry + 1):  # >= 10 sec wait
+        try:
+            try:
+                with open(filename, 'r') as fp:
+                    data = json.load(fp)
+                    break
+            except JSONDecodeError:
+                lgr.error("{fname} is not a valid json file".format(fname=filename))
+                raise
+        except (JSONDecodeError, FileNotFoundError) as exc:
+            if i >= retry:
+                raise
+            lgr.warning("Caught %s. Will retry again", exc)
+            sleep(0.1)
+            continue
 
     return data