From b5d79b4c4444872b1819f5ebcbd42362bf7974bb Mon Sep 17 00:00:00 2001 From: anibalsolon Date: Wed, 27 Jun 2018 20:58:45 -0400 Subject: [PATCH 1/3] add versioning to crash files --- nipype/pipeline/plugins/tools.py | 3 +- nipype/utils/filemanip.py | 59 ++++++++++++++++++++++++---- nipype/utils/tests/test_filemanip.py | 51 +++++++++++++++++++++++- 3 files changed, 103 insertions(+), 10 deletions(-) diff --git a/nipype/pipeline/plugins/tools.py b/nipype/pipeline/plugins/tools.py index 79922bf998..9a214053c7 100644 --- a/nipype/pipeline/plugins/tools.py +++ b/nipype/pipeline/plugins/tools.py @@ -58,7 +58,8 @@ def report_crash(node, traceback=None, hostname=None): if crashfile.endswith('.txt'): crash2txt(crashfile, dict(node=node, traceback=traceback)) else: - savepkl(crashfile, dict(node=node, traceback=traceback)) + savepkl(crashfile, dict(node=node, traceback=traceback), + versioning=True) return crashfile diff --git a/nipype/utils/filemanip.py b/nipype/utils/filemanip.py index c7458fa5d8..dec902b2da 100644 --- a/nipype/utils/filemanip.py +++ b/nipype/utils/filemanip.py @@ -628,13 +628,13 @@ def load_json(filename): def loadcrash(infile, *args): - if '.pkl' in infile: - return loadpkl(infile) + if infile.endswith('pkl') or infile.endswith('pklz'): + return loadpkl(infile, versioning=True) else: raise ValueError('Only pickled crashfiles are supported') -def loadpkl(infile): +def loadpkl(infile, versioning=False): """Load a zipped or plain cPickled file """ fmlogger.debug('Loading pkl: %s', infile) @@ -643,11 +643,44 @@ def loadpkl(infile): else: pkl_file = open(infile, 'rb') + if versioning: + pkl_metadata = {} + + # Look if pkl file contains version file + try: + pkl_metadata_line = pkl_file.readline() + pkl_metadata = json.loads(pkl_metadata_line) + except: + # Could not get version info + pkl_file.seek(0) + try: - unpkl = pickle.load(pkl_file) - except UnicodeDecodeError: - unpkl = pickle.load(pkl_file, fix_imports=True, encoding='utf-8') - return unpkl + try: + unpkl = pickle.load(pkl_file) + except UnicodeDecodeError: + unpkl = pickle.load(pkl_file, fix_imports=True, encoding='utf-8') + + return unpkl + + # Unpickling problems + except Exception as e: + if not versioning: + return None + + from nipype import __version__ as version + + if 'version' in pkl_metadata: + if pkl_metadata['version'] != version: + fmlogger.error('Your Nipype version is: %s', + version) + fmlogger.error('Nipype version of the pkl is: %s', + pkl_metadata['version']) + else: + fmlogger.error('No metadata was found in the pkl file.') + fmlogger.error('Make sure that you are using the same Nipype' + 'version from the generated pkl.') + + raise e def crash2txt(filename, record): @@ -682,11 +715,21 @@ def read_stream(stream, logger=None, encoding=None): return out.splitlines() -def savepkl(filename, record): +def savepkl(filename, record, versioning=False): if filename.endswith('pklz'): pkl_file = gzip.open(filename, 'wb') else: pkl_file = open(filename, 'wb') + + if versioning: + from nipype import __version__ as version + metadata = json.dumps({'version': version}, + ensure_ascii=True, + encoding='ascii') + + pkl_file.write(metadata.encode('ascii')) + pkl_file.write('\n'.encode('ascii')) + pickle.dump(record, pkl_file) pkl_file.close() diff --git a/nipype/utils/tests/test_filemanip.py b/nipype/utils/tests/test_filemanip.py index 1ab5f4af0b..ae5316c7d7 100644 --- a/nipype/utils/tests/test_filemanip.py +++ b/nipype/utils/tests/test_filemanip.py @@ -8,13 +8,15 @@ import time import warnings +import mock import pytest from ...testing import TempFATFS from ...utils.filemanip import ( save_json, load_json, fname_presuffix, fnames_presuffix, hash_rename, check_forhash, _parse_mount_table, _cifs_table, on_cifs, copyfile, copyfiles, ensure_list, simplify_list, check_depends, - split_filename, get_related_files, indirectory) + split_filename, get_related_files, indirectory, + loadpkl, loadcrash, savepkl) def _ignore_atime(stat): @@ -521,3 +523,50 @@ def test_indirectory(tmpdir): except ValueError: pass assert os.getcwd() == tmpdir.strpath + + +def test_pklization(tmpdir): + tmpdir.chdir() + + exc = Exception("There is something wrong here") + savepkl('./except.pkz', exc) + newexc = loadpkl('./except.pkz') + + assert exc.args == newexc.args + assert os.getcwd() == tmpdir.strpath + + +class Pickled: + + def __getstate__(self): + return self.__dict__ + + +class PickledBreaker: + + def __setstate__(self, d): + raise Exception() + + +def test_versioned_pklization(tmpdir): + tmpdir.chdir() + + obj = Pickled() + savepkl('./pickled.pkz', obj, versioning=True) + + with pytest.raises(Exception): + with mock.patch('nipype.utils.tests.test_filemanip.Pickled', PickledBreaker), \ + mock.patch('nipype.__version__', '0.0.0'): + + loadpkl('./pickled.pkz', versioning=True) + + +def test_unversioned_pklization(tmpdir): + tmpdir.chdir() + + obj = Pickled() + savepkl('./pickled.pkz', obj) + + with pytest.raises(Exception): + with mock.patch('nipype.utils.tests.test_filemanip.Pickled', PickledBreaker): + loadpkl('./pickled.pkz', versioning=True) From 8c8e7d8af0a08d407b2eb2f2647ef45ce15abe05 Mon Sep 17 00:00:00 2001 From: anibalsolon Date: Thu, 28 Jun 2018 10:40:42 -0400 Subject: [PATCH 2/3] raise error, instead of return None, to keep it retrocompatible --- nipype/utils/filemanip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/utils/filemanip.py b/nipype/utils/filemanip.py index dec902b2da..4fa855c5fb 100644 --- a/nipype/utils/filemanip.py +++ b/nipype/utils/filemanip.py @@ -665,7 +665,7 @@ def loadpkl(infile, versioning=False): # Unpickling problems except Exception as e: if not versioning: - return None + raise e from nipype import __version__ as version From d3de9f46688844537107bec7510addeff9f98886 Mon Sep 17 00:00:00 2001 From: anibalsolon Date: Fri, 29 Jun 2018 15:13:57 -0400 Subject: [PATCH 3/3] use utf-8 encoding for JSON metadata --- nipype/utils/filemanip.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/nipype/utils/filemanip.py b/nipype/utils/filemanip.py index 4fa855c5fb..480fdbd6ca 100644 --- a/nipype/utils/filemanip.py +++ b/nipype/utils/filemanip.py @@ -723,12 +723,10 @@ def savepkl(filename, record, versioning=False): if versioning: from nipype import __version__ as version - metadata = json.dumps({'version': version}, - ensure_ascii=True, - encoding='ascii') + metadata = json.dumps({'version': version}) - pkl_file.write(metadata.encode('ascii')) - pkl_file.write('\n'.encode('ascii')) + pkl_file.write(metadata.encode('utf-8')) + pkl_file.write('\n'.encode('utf-8')) pickle.dump(record, pkl_file) pkl_file.close()