diff --git a/README.md b/README.md index 45ed525..0b1ce3b 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,9 @@ to provide the license to this gear. A license is required for this gear to run ### fs-subjects-dir (optional) Zip file of existing FreeSurfer subject's directory to reuse. If the output of FreeSurfer recon-all is provided to fMRIPrep, that output will be used rather than re-running recon-all. Unzipping the file should produce a particular subject's directory which will be placed in the $FREESURFER_HOME/subjects directory. The name of the directory must match the -subjid as passed to recon-all. This version of fMRIPrep uses Freesurfer v6.0.1. +### previous-results (optional) +Provide previously calculated fMRIPrep output results zip file as in input. This file will be unzipped into the output directory so that previous results will be used instead of re-calculating them. This input is provided so that bids-fmriprep can be run incrementally as new data is acquired. + ### work-dir (optional) THIS IS A FUTURE OPTIONAL INPUT. It has not yet been added. Provide intermediate fMRIPrep results as a zip file. This file will be unzipped into the work directory so that previous results will be used instead of re-calculating them. This option is provided so that bids-fmriprep can be run incrementally as new data is acquired. The zip file to provide can be produced by using the gear-save-intermediate-output configuration option. You definitely also want to use the fs-subject-dir input (above) so that FreeSurfer won't be run multiple times. diff --git a/manifest.json b/manifest.json index f4f052c..64916a5 100644 --- a/manifest.json +++ b/manifest.json @@ -156,6 +156,11 @@ "description": "Gear will save ALL intermediate output into fmriprep_work_*.zip", "type": "boolean" }, + "gear-writable-dir": { + "default": "/var/tmp", + "description": "Gears expect to be able to write temporary files in /flywheel/v0/. If this location is not writable (such as when running in Singularity), this path will be used instead. fMRIPrep creates a large number of files so this disk space should be fast and local.", + "type": "string" + }, "ignore": { "default": "", "description": "Ignore selected aspects of the input dataset to disable corresponding parts of the workflow (a space delimited list) Possible choices: fieldmaps, slicetiming, sbref", @@ -303,13 +308,13 @@ } }, "custom": { - "docker-image": "flywheel/bids-fmriprep:1.2.4_20.2.6", + "docker-image": "flywheel/bids-fmriprep:1.2.8_20.2.6", "flywheel": { "suite": "BIDS Apps" }, "gear-builder": { "category": "analysis", - "image": "flywheel/bids-fmriprep:1.2.4_20.2.6" + "image": "flywheel/bids-fmriprep:1.2.8_20.2.6" }, "license": { "dependencies": [ @@ -383,5 +388,5 @@ "name": "bids-fmriprep", "source": "https://github.com/nipreps/fmriprep", "url": "https://github.com/flywheel-apps/bids-fmriprep/blob/master/README.md", - "version": "1.2.4_20.2.6" + "version": "1.2.8_20.2.6" } diff --git a/run.py b/run.py index 9241eff..c46e4cc 100755 --- a/run.py +++ b/run.py @@ -190,58 +190,6 @@ def main(gtk_context): log.info("Using provided PyBIDS filter file %s", str(paths[0])) config["bids-filter-file"] = str(paths[0]) - previous_work_zip_file_path = gtk_context.get_input_path("work-dir") - if previous_work_zip_file_path: - paths = list(Path("input/work-dir").glob("*")) - log.info("Using provided fMRIPrep intermediate work file %s", str(paths[0])) - unzip_dir = FWV0 / "unzip-work-dir" - unzip_dir.mkdir(parents=True) - unzip_archive(paths[0], unzip_dir) - for a_dir in unzip_dir.glob("*/*"): - if ( - a_dir.name == "bids" - ): # skip previous bids directory so current bids data will be used - log.info( - "Found %s, but ignoring it to use current bids data", a_dir.name - ) - else: - log.info("Found %s", a_dir.name) - a_dir.rename(FWV0 / "work" / a_dir.name) - hash_file = list(Path("work/fmriprep_wf/").glob("fsdir_run_*/_0x*.json"))[0] - if hash_file.exists(): - with open(hash_file) as json_file: - data = json.load(json_file) - old_tmp_path = data[0][1] - old_tmp_name = old_tmp_path.split("/")[2] - log.info("Found old tmp name: %s", old_tmp_name) - cur_tmp_name = str(FWV0).split("/")[2] - # rename the directory to the old name - Path("/tmp/" + cur_tmp_name).replace(Path("/tmp/" + old_tmp_name)) - # create a symbolic link using the new name to the old name just in case - Path("/tmp/" + cur_tmp_name).symlink_to( - Path("/tmp/" + old_tmp_name), target_is_directory=True - ) - # update all variables to have the old directory name in them - FWV0 = Path("/tmp/" + old_tmp_name + "/flywheel/v0") - output_dir = str(output_dir).replace(cur_tmp_name, old_tmp_name) - output_analysis_id_dir = Path( - str(output_analysis_id_dir).replace(cur_tmp_name, old_tmp_name) - ) - log.info("new output directory is: %s", output_dir) - work_dir = Path(str(work_dir).replace(cur_tmp_name, old_tmp_name)) - log.info("new work directory is: %s", work_dir) - subjects_dir = Path( - str(subjects_dir).replace(cur_tmp_name, old_tmp_name) - ) - config["fs-subjects-dir"] = subjects_dir - log.info("new FreeSurfer subjects directory is: %s", subjects_dir) - # for old work to be recognized, switch to running from the old path - os.chdir(FWV0) - log.info("cd %s", FWV0) - else: - log.info("Could not find hash file") - config["work-dir"] = str(FWV0 / "work") - subject_zip_file_path = gtk_context.get_input_path("fs-subjects-dir") if subject_zip_file_path: paths = list(Path("input/fs-subjects-dir").glob("*")) @@ -283,11 +231,17 @@ def main(gtk_context): FREESURFER_LICENSE, ) - # TemplateFlow seems to be baked in to the container since 2021-10-07 16:25:12 so this is not needed - # templateflow_dir = FWV0 / "templateflow" - # templateflow_dir.mkdir() - # environ["SINGULARITYENV_TEMPLATEFLOW_HOME"] = str(templateflow_dir) - # environ["TEMPLATEFLOW_HOME"] = str(templateflow_dir) + # TemplateFlow seems to be baked in to the container since 2021-10-07 16:25:12 so this is not needed...actually, it is for now... + templateflow_dir = FWV0 / "templateflow" + templateflow_dir.mkdir() + environ["SINGULARITYENV_TEMPLATEFLOW_HOME"] = str(templateflow_dir) + environ["TEMPLATEFLOW_HOME"] = str(templateflow_dir) + orig = Path("/home/fmriprep/.cache/templateflow/") + # Fill writable templateflow directory with existing templates so they don't have to be downloaded + templates = list(orig.glob("*")) + for tt in templates: + # (templateflow_dir / tt.name).symlink_to(tt) + shutil.copytree(tt, templateflow_dir / tt.name) command = generate_command( config, work_dir, output_analysis_id_dir, errors, warnings @@ -317,140 +271,164 @@ def main(gtk_context): log.info("Did not download BIDS because of previous errors") print(errors) - # Don't run if there were errors or if this is a dry run return_code = 0 + num_tries = 0 - try: + # Don't run if there were errors or if this is a dry run + if len(errors) > 0: + return_code = 1 + log.info("Command was NOT run because of previous errors.") + num_tries == 2 # don't try to run - if len(errors) > 0: - return_code = 1 - log.info("Command was NOT run because of previous errors.") + while num_tries < 2: - elif dry_run: - e = "gear-dry-run is set: Command was NOT run." - log.warning(e) - warnings.append(e) - pretend_it_ran(destination_id) + try: - else: - if config["gear-log-level"] != "INFO": - # show what's in the current working directory just before running - os.system("tree -al .") + num_tries += 1 + if num_tries > 1: + log.info("Trying a second time") - if "gear-timeout" in config: - command = [f"timeout {config['gear-timeout']}"] + command + if dry_run: + e = "gear-dry-run is set: Command was NOT run." + log.warning(e) + warnings.append(e) + pretend_it_ran(destination_id) - # This is what it is all about - exec_command( - command, environ=environ, dry_run=dry_run, shell=True, cont_output=True, - ) + else: - except RuntimeError as exc: - return_code = 1 - errors.append(exc) - log.critical(exc) - log.exception("Unable to execute command.") - - finally: - - # Save time, etc. resources used in metadata on analysis - if Path("time_output.txt").exists(): # some tests won't have this file - metadata = { - "analysis": {"info": {"resources used": {},},}, - } - with open("time_output.txt") as file: - for line in file: - if ":" in line: - if ( - "Elapsed" in line - ): # special case "Elapsed (wall clock) time (h:mm:ss or m:ss): 0:08.11" - sline = re.split(r"\):", line) - sline[0] += ")" - else: - sline = line.split(":") - key = sline[0].strip() - val = sline[1].strip(' "\n') - metadata["analysis"]["info"]["resources used"][key] = val - with open(f"{output_dir}/.metadata.json", "w") as fff: - json.dump(metadata, fff) - log.info(f"Wrote {output_dir}/.metadata.json") - - # Cleanup, move all results to the output directory - - # Remove all fsaverage* directories - if not config.get("gear-keep-fsaverage"): - path = output_analysis_id_dir / "freesurfer" - fsavg_dirs = path.glob("fsaverage*") - for fsavg in fsavg_dirs: - log.info("deleting %s", str(fsavg)) - shutil.rmtree(fsavg) - else: - log.info("Keeping fsaverage directories") - - # zip entire output/ folder into - # __.zip - zip_file_name = gear_name + f"_{run_label}_{destination_id}.zip" - zip_output( - str(output_dir), - destination_id, - zip_file_name, - dry_run=False, - exclude_files=None, - ) + if "gear-timeout" in config: + command = [f"timeout {config['gear-timeout']}"] + command + + if config["gear-log-level"] != "INFO": + # show what's in the current working directory just before running + os.system("tree -alh .") + + # This is what it is all about + exec_command( + command, + environ=environ, + dry_run=dry_run, + shell=True, + cont_output=True, + ) - # Make archives for result *.html files for easy display on platform - zip_htmls(output_dir, destination_id, output_analysis_id_dir / BIDS_APP) - - # possibly save ALL intermediate output - if config.get("gear-save-intermediate-output"): - zip_all_intermediate_output( - destination_id, gear_name, output_dir, work_dir, run_label - ) - - # possibly save intermediate files and folders - zip_intermediate_selected( - config.get("gear-intermediate-files"), - config.get("gear-intermediate-folders"), - destination_id, - gear_name, - output_dir, - work_dir, - run_label, + except RuntimeError as exc: + return_code = 1 + errors.append(exc) + log.critical(exc) + log.exception("Unable to execute command.") + + # Save time, etc. resources used in metadata on analysis + if Path("time_output.txt").exists(): # some tests won't have this file + metadata = { + "analysis": {"info": {"resources used": {},},}, + } + with open("time_output.txt") as file: + for line in file: + if ":" in line: + if ( + "Elapsed" in line + ): # special case "Elapsed (wall clock) time (h:mm:ss or m:ss): 0:08.11" + sline = re.split(r"\):", line) + sline[0] += ")" + else: + sline = line.split(":") + key = sline[0].strip() + val = sline[1].strip(' "\n') + metadata["analysis"]["info"]["resources used"][key] = val + with open(f"{output_dir}/.metadata.json", "w") as fff: + json.dump(metadata, fff) + log.info(f"Wrote {output_dir}/.metadata.json") + + # Cleanup, move all results to the output directory + + if return_code != 0: + os.system("echo ") + os.system("echo Disk Information on Failure") + os.system("df -h") + + # Remove all fsaverage* directories + if not config.get("gear-keep-fsaverage"): + path = output_analysis_id_dir / "freesurfer" + fsavg_dirs = path.glob("fsaverage*") + for fsavg in fsavg_dirs: + log.info("deleting %s", str(fsavg)) + shutil.rmtree(fsavg) + else: + log.info("Keeping fsaverage directories") + + # zip entire output/ folder into + # __.zip + zip_file_name = gear_name + f"_{run_label}_{destination_id}.zip" + zip_output( + str(output_dir), + destination_id, + zip_file_name, + dry_run=False, + exclude_files=None, + ) + + # Make archives for result *.html files for easy display on platform + zip_htmls(output_dir, destination_id, output_analysis_id_dir / BIDS_APP) + + # possibly save ALL intermediate output + if config.get("gear-save-intermediate-output"): + zip_all_intermediate_output( + destination_id, gear_name, output_dir, work_dir, run_label ) - # clean up: remove output that was zipped - if Path(output_analysis_id_dir).exists(): - if not config.get("gear-keep-output"): + # possibly save intermediate files and folders + zip_intermediate_selected( + config.get("gear-intermediate-files"), + config.get("gear-intermediate-folders"), + destination_id, + gear_name, + output_dir, + work_dir, + run_label, + ) - log.debug('removing output directory "%s"', str(output_analysis_id_dir)) - shutil.rmtree(output_analysis_id_dir) + # clean up: remove output that was zipped + if Path(output_analysis_id_dir).exists(): + if not config.get("gear-keep-output"): - else: - log.info( - 'NOT removing output directory "%s"', str(output_analysis_id_dir) - ) + log.debug('removing output directory "%s"', str(output_analysis_id_dir)) + shutil.rmtree(output_analysis_id_dir) else: - log.info("Output directory does not exist so it cannot be removed") - - # Report errors and warnings at the end of the log so they can be easily seen. - if len(warnings) > 0: - msg = "Previous warnings:\n" - for warn in warnings: - msg += " Warning: " + str(warn) + "\n" - log.info(msg) - - if len(errors) > 0: - msg = "Previous errors:\n" - for err in errors: - if str(type(err)).split("'")[1] == "str": - # show string - msg += " Error msg: " + str(err) + "\n" - else: # show type (of error) and error message - err_type = str(type(err)).split("'")[1] - msg += f" {err_type}: {str(err)}\n" - log.info(msg) - return_code = 1 + log.info('NOT removing output directory "%s"', str(output_analysis_id_dir)) + + else: + log.info("Output directory does not exist so it cannot be removed") + + # Report errors and warnings at the end of the log so they can be easily seen. + if len(warnings) > 0: + msg = "Previous warnings:\n" + for warn in warnings: + msg += " Warning: " + str(warn) + "\n" + log.info(msg) + + if len(errors) > 0: + msg = "Previous errors:\n" + for err in errors: + if str(type(err)).split("'")[1] == "str": + # show string + msg += " Error msg: " + str(err) + "\n" + else: # show type (of error) and error message + err_type = str(type(err)).split("'")[1] + msg += f" {err_type}: {str(err)}\n" + log.info(msg) + return_code = 1 + + if num_tries == 1: + log.info("Happily, fMRIPrep worked on the first try.") + else: + msg = ( + "first try but it did on the second" + if return_code == 0 + else "first or second try" + ) + log.info("Sadly, fMRIPrep did not work on the %s.", msg) log.info("%s Gear is done. Returning %s", CONTAINER, return_code) @@ -459,18 +437,21 @@ def main(gtk_context): if __name__ == "__main__": - # always run in a newly created "scratch" directory in /tmp/... - scratch_dir = run_in_tmp_dir() - with flywheel_gear_toolkit.GearToolkitContext() as gtk_context: + + # make sure /flywheel/v0 is writable, use a scratch directory if not + scratch_dir = run_in_tmp_dir(gtk_context.config["gear-writable-dir"]) + return_code = main(gtk_context) - # clean up (might be necessary when running in a shared computing environment) - for thing in scratch_dir.glob("*"): - if thing.is_symlink(): - thing.unlink() # don't remove anything links point to - log.debug("unlinked %s", thing.name) - shutil.rmtree(scratch_dir) - log.debug("Removed %s", scratch_dir) + # clean up (might be necessary when running in a shared computing environment) + if scratch_dir: + log.debug("Removing scratch directory") + for thing in scratch_dir.glob("*"): + if thing.is_symlink(): + thing.unlink() # don't remove anything links point to + log.debug("unlinked %s", thing.name) + shutil.rmtree(scratch_dir) + log.debug("Removed %s", scratch_dir) sys.exit(return_code) diff --git a/tests/conftest.py b/tests/conftest.py index 574ec35..c73cc5c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,7 @@ from utils.singularity import run_in_tmp_dir -run_in_tmp_dir() # run all tests in /tmp/*/flywheel/v0 where * is random +run_in_tmp_dir("/var/tmp") # run all tests in /tmp/*/flywheel/v0 where * is random FWV0 = Path.cwd() diff --git a/tests/integration_tests/test_dry_run.py b/tests/integration_tests/test_dry_run.py index 58f0242..d9e3bb2 100644 --- a/tests/integration_tests/test_dry_run.py +++ b/tests/integration_tests/test_dry_run.py @@ -42,27 +42,6 @@ def test_dry_run_works( "command is", "--bids-filter-file=input/bids-filter-file/PyBIDS_filter.json", ) - assert search_caplog_contains( - caplog, "--work-dir", "flywheel/v0/output/61608fc7dbf5f9487f231006" - ) - - # Since the "work-dir" option was used, the gear created the old data's /tmp/ path and - # ran in it, and a symbolic link was created from the current /tmp/ path to the old one. - # The "current" path was created just before these tests started (in conftest.py). E.g.: - # % ls /tmp - # gear-temp-dir-2cde80oy -> /tmp/gear-temp-dir-yhv7hq29 - # gear-temp-dir-yhv7hq29 - # where "yhv7hq29" is the old random part found in the provided "work-dir" data and - # "2cde80oy" is the new random part of the /tmp/ path created to run these tests. - # The new part is generated randomly so it will change but the old one is in the gear test - # data provided as an input to the gear. - old_tmp_path = Path(*(list(Path.cwd().parts)[:3])) - assert "yhv7hq29" in list(old_tmp_path.parts)[2] - current_tmp_path = Path(*(list(FWV0.parts)[:3])) - assert current_tmp_path.is_symlink() - # now change back to the original /tmp/ path so that following tests will not be affected - current_tmp_path.unlink() - old_tmp_path.replace(current_tmp_path) assert search_caplog_contains(caplog, "command is", "participant") assert search_caplog_contains(caplog, "command is", "'arg1', 'arg2'") diff --git a/tests/integration_tests/test_fake_data_timeout.py b/tests/integration_tests/test_fake_data_timeout.py index cc852d8..3eb2287 100644 --- a/tests/integration_tests/test_fake_data_timeout.py +++ b/tests/integration_tests/test_fake_data_timeout.py @@ -30,8 +30,23 @@ def test_fake_data_killed( with flywheel_gear_toolkit.GearToolkitContext(input_args=[]) as gtk_context: + Path("/home/fmriprep/.cache/templateflow/").chmod(0o555) + status = run.main(gtk_context) + assert Path( + "/flywheel/v0/templateflow/tpl-MNI152NLin2009cAsym/tpl-MNI152NLin2009cAsym_res-02_label-WM_probseg.nii.gz" + ).is_file() + + assert ( + Path( + "/flywheel/v0/templateflow/tpl-OASIS30ANTs/tpl-OASIS30ANTs_res-01_label-WM_probseg.nii.gz" + ) + .stat() + .st_size + != 0 + ) + toml_file = list(FWV0.glob("work/*/config.toml"))[0] assert toml_file.exists() toml_info = toml.load(toml_file) @@ -46,3 +61,6 @@ def test_fake_data_killed( # assert toml_info["execution"]["templateflow_home"] == str(FWV0 / "templateflow") assert search_caplog(caplog, "Unable to execute command") + assert search_caplog( + caplog, "Sadly, fMRIPrep did not work on the first or second try" + ) diff --git a/tests/unit_tests/test_download_run_level.py b/tests/unit_tests/test_download_run_level.py index 531502a..778d57f 100644 --- a/tests/unit_tests/test_download_run_level.py +++ b/tests/unit_tests/test_download_run_level.py @@ -103,7 +103,7 @@ def __init__(self): self.label = "TheAcquisitionLabel" -def test_download_bids_for_runlevel_acquisition_works(tmp_path, caplog): +def test_download_bids_for_runlevel_acquisition_works(tmp_path, search_caplog, caplog): caplog.set_level(logging.DEBUG) @@ -137,8 +137,7 @@ def test_download_bids_for_runlevel_acquisition_works(tmp_path, caplog): dry_run=False, ) - assert len(caplog.records) == 9 - assert "Downloading BIDS data was successful" in caplog.records[8].message + assert search_caplog(caplog, "Downloading BIDS data was successful") def test_download_bids_for_runlevel_no_destination_complains(tmp_path, caplog): @@ -171,14 +170,15 @@ def test_download_bids_for_runlevel_no_destination_complains(tmp_path, caplog): dry_run=True, ) - assert len(caplog.records) == 2 assert "Destination does not exist" in caplog.records[0].message HIERARCHY["run_level"] = "acquisition" # fix what was broke HIERARCHY["run_label"] = HIERARCHY[f"{HIERARCHY['run_level']}_label"] -def test_download_bids_for_runlevel_bad_destination_noted(tmp_path, caplog): +def test_download_bids_for_runlevel_bad_destination_noted( + tmp_path, caplog, search_caplog +): caplog.set_level(logging.DEBUG) @@ -218,15 +218,16 @@ def test_download_bids_for_runlevel_bad_destination_noted(tmp_path, caplog): dry_run=True, ) - assert len(caplog.records) == 9 - assert "is not an analysis or acquisition" in caplog.records[0].message - assert 'subject "TheSubjectCode"' in caplog.records[4].message + assert search_caplog(caplog, "is not an analysis or acquisition") + assert search_caplog(caplog, 'subject "TheSubjectCode"') HIERARCHY["run_level"] = "acquisition" # fix what was broke HIERARCHY["run_label"] = HIERARCHY[f"{HIERARCHY['run_level']}_label"] -def test_download_bids_for_runlevel_unknown_acquisition_detected(tmp_path, caplog): +def test_download_bids_for_runlevel_unknown_acquisition_detected( + tmp_path, caplog, search_caplog +): caplog.set_level(logging.DEBUG) @@ -266,11 +267,10 @@ def test_download_bids_for_runlevel_unknown_acquisition_detected(tmp_path, caplo dry_run=True, ) - assert len(caplog.records) == 5 - assert 'acquisition "unknown acquisition"' in caplog.records[3].message + assert search_caplog(caplog, 'acquisition "unknown acquisition"') -def test_download_bids_for_runlevel_session_works(tmp_path, caplog): +def test_download_bids_for_runlevel_session_works(tmp_path, caplog, search_caplog): caplog.set_level(logging.DEBUG) @@ -310,14 +310,15 @@ def test_download_bids_for_runlevel_session_works(tmp_path, caplog): dry_run=True, ) - assert len(caplog.records) == 8 - assert 'session "TheSessionLabel"' in caplog.records[3].message + assert search_caplog(caplog, 'session "TheSessionLabel"') HIERARCHY["run_level"] = "acquisition" # fix what was broke HIERARCHY["run_label"] = HIERARCHY[f"{HIERARCHY['run_level']}_label"] -def test_download_bids_for_runlevel_acquisition_exception_detected(tmp_path, caplog): +def test_download_bids_for_runlevel_acquisition_exception_detected( + tmp_path, caplog, search_caplog +): caplog.set_level(logging.DEBUG) @@ -348,11 +349,10 @@ def test_download_bids_for_runlevel_acquisition_exception_detected(tmp_path, cap dry_run=True, ) - assert len(caplog.records) == 6 - assert "(foo) Reason: fum" in caplog.records[4].message + assert search_caplog(caplog, "(foo) Reason: fum") -def test_download_bids_for_runlevel_unknown_detected(tmp_path, caplog): +def test_download_bids_for_runlevel_unknown_detected(tmp_path, caplog, search_caplog): caplog.set_level(logging.DEBUG) @@ -386,15 +386,14 @@ def test_download_bids_for_runlevel_unknown_detected(tmp_path, caplog): dry_run=True, ) - assert len(caplog.records) == 5 - assert "run_level = who knows" in caplog.records[3].message + assert search_caplog(caplog, "run_level = who knows") HIERARCHY["run_level"] = "acquisition" # fix what was broke HIERARCHY["run_label"] = HIERARCHY[f"{HIERARCHY['run_level']}_label"] def test_download_bids_for_runlevel_bidsexporterror_exception_detected( - tmp_path, caplog + tmp_path, caplog, search_caplog ): caplog.set_level(logging.DEBUG) @@ -426,11 +425,12 @@ def test_download_bids_for_runlevel_bidsexporterror_exception_detected( dry_run=True, ) - assert len(caplog.records) == 6 - assert "crash" in caplog.records[4].message + assert search_caplog(caplog, "crash") -def test_download_bids_for_runlevel_validate_exception_detected(tmp_path, caplog): +def test_download_bids_for_runlevel_validate_exception_detected( + tmp_path, caplog, search_caplog +): caplog.set_level(logging.DEBUG) @@ -470,14 +470,15 @@ def test_download_bids_for_runlevel_validate_exception_detected(tmp_path, caplog dry_run=True, ) - assert len(caplog.records) == 9 - assert "('except', 'what')" in caplog.records[7].message + assert search_caplog(caplog, "('except', 'what')") HIERARCHY["run_level"] = "acquisition" # fix what was broke HIERARCHY["run_label"] = HIERARCHY[f"{HIERARCHY['run_level']}_label"] -def test_download_bids_for_runlevel_nothing_downloaded_detected(tmp_path, caplog): +def test_download_bids_for_runlevel_nothing_downloaded_detected( + tmp_path, caplog, search_caplog +): caplog.set_level(logging.DEBUG) @@ -506,13 +507,7 @@ def test_download_bids_for_runlevel_nothing_downloaded_detected(tmp_path, caplog dry_run=True, ) - assert len(caplog.records) == 6 - assert "No BIDS data was found" in caplog.records[4].message + assert search_caplog(caplog, "No BIDS data was found") HIERARCHY["run_level"] = "acquisition" # fix what was broke HIERARCHY["run_label"] = HIERARCHY[f"{HIERARCHY['run_level']}_label"] - - -if __name__ == "__main__": - - test_download_bids_for_runlevel_project_works("/tmp", None) diff --git a/tests/unit_tests/test_environment.py b/tests/unit_tests/test_environment.py index 072b498..d236639 100644 --- a/tests/unit_tests/test_environment.py +++ b/tests/unit_tests/test_environment.py @@ -14,4 +14,6 @@ def test_get_and_log_environment_works(caplog, search_caplog): assert environ["FLYWHEEL"] == "/flywheel/v0" assert environ["FREESURFER_HOME"] == "/opt/freesurfer" assert search_caplog(caplog, "FREESURFER_HOME=/opt/freesurfer") - assert search_caplog(caplog, "Grabbing environment from /tmp/gear-temp-dir-") + assert search_caplog( + caplog, "Grabbing environment from /flywheel/v0/gear_environ.json" + ) diff --git a/utils/singularity.py b/utils/singularity.py index dea3b4b..208b395 100644 --- a/utils/singularity.py +++ b/utils/singularity.py @@ -15,9 +15,13 @@ SCRATCH_NAME = "gear-temp-dir-" -def run_in_tmp_dir(): +def run_in_tmp_dir(writable_dir): """Copy gear to a temporary directory and cd to there. + Args: + writable_dir (string): directory to use for temporary files if /flywheel/v0 is not + writable. + Returns: tmp_path (path) The path to the temporary directory so it can be deleted """ @@ -43,12 +47,20 @@ def run_in_tmp_dir(): else: log.debug("Running in %s", running_in) + try: + _ = tempfile.mkdtemp(prefix=SCRATCH_NAME, dir=FWV0) + os.chdir(FWV0) # run in /tmp/... directory so it is writeable + log.debug("Running in %s", FWV0) + return None + except OSError as e: + log.debug("Problem writing to %s: %s", FWV0, e.strerror) + # This used to remove any previous runs (possibly left over from previous testing) but that would be bad # if other bids-fmripreps are running on shared hardware at the same time because their directories would # be deleted mid-run. A very confusing error to debug! # Create temporary place to run gear - WD = tempfile.mkdtemp(prefix=SCRATCH_NAME, dir="/tmp") + WD = tempfile.mkdtemp(prefix=SCRATCH_NAME, dir=writable_dir) log.debug("Gear scratch directory is %s", WD) new_FWV0 = Path(WD + FWV0)