From 09f51f59af9290cd4c93345c6e0f682d32af052e Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 13 Dec 2017 08:39:00 -0500 Subject: [PATCH 1/4] Upload optimization - eliminate second call to check_binary in upload.py. --- tools/data_source/upload.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py index 7e9d06a1ebf..2c229ba2890 100644 --- a/tools/data_source/upload.py +++ b/tools/data_source/upload.py @@ -123,7 +123,8 @@ def add_file(dataset, registry, json_file, output_path): file_err('The uploaded file is empty', dataset, json_file) return # Is dataset content supported sniffable binary? - if check_binary(dataset.path): + is_binary = check_binary(dataset.path) + if is_binary: # Sniff the data type guessed_ext = sniff.guess_ext(dataset.path, registry.sniff_order) # Set data_type only if guessed_ext is a binary datatype @@ -259,7 +260,7 @@ def add_file(dataset, registry, json_file, output_path): dataset.name = uncompressed_name data_type = 'zip' if not data_type: - if check_binary(dataset.path) or registry.is_extension_unsniffable_binary(dataset.file_type): + if is_binary or registry.is_extension_unsniffable_binary(dataset.file_type): # We have a binary dataset, but it is not Bam, Sff or Pdf data_type = 'binary' parts = dataset.name.split(".") From cafac19f65020ef29cd8d73d226c43e901318616 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 13 Dec 2017 12:57:08 -0500 Subject: [PATCH 2/4] Upload refactor - make link_data_only a bool. Since it is a bool. --- tools/data_source/upload.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py index 2c229ba2890..0417d1944cb 100644 --- a/tools/data_source/upload.py +++ b/tools/data_source/upload.py @@ -76,7 +76,7 @@ def add_file(dataset, registry, json_file, output_path): line_count = None converted_path = None stdout = None - link_data_only = dataset.get('link_data_only', 'copy_files') + link_data_only = dataset.get('link_data_only', 'copy_files') != 'copy_files' # run_as_real_user is estimated from galaxy config (external chmod indicated of inputs executed) # If this is True we always purge supplied upload inputs so they are cleaned up and we reuse their @@ -144,7 +144,7 @@ def add_file(dataset, registry, json_file, output_path): file_err('The gzipped uploaded file contains inappropriate content', dataset, json_file) return elif is_gzipped and is_valid and auto_decompress: - if link_data_only == 'copy_files': + if not link_data_only: # We need to uncompress the temp_name file, but BAM files must remain compressed in the BGZF format CHUNK_SIZE = 2 ** 20 # 1Mb fd, uncompressed = tempfile.mkstemp(prefix='data_id_%s_upload_gunzip_' % dataset.dataset_id, dir=os.path.dirname(output_path), text=False) @@ -177,7 +177,7 @@ def add_file(dataset, registry, json_file, output_path): file_err('The gzipped uploaded file contains inappropriate content', dataset, json_file) return elif is_bzipped and is_valid and auto_decompress: - if link_data_only == 'copy_files': + if not link_data_only: # We need to uncompress the temp_name file CHUNK_SIZE = 2 ** 20 # 1Mb fd, uncompressed = tempfile.mkstemp(prefix='data_id_%s_upload_bunzip2_' % dataset.dataset_id, dir=os.path.dirname(output_path), text=False) @@ -207,7 +207,7 @@ def add_file(dataset, registry, json_file, output_path): # See if we have a zip archive is_zipped = check_zip(dataset.path) if is_zipped and auto_decompress: - if link_data_only == 'copy_files': + if not link_data_only: CHUNK_SIZE = 2 ** 20 # 1Mb uncompressed = None uncompressed_name = None @@ -280,7 +280,7 @@ def add_file(dataset, registry, json_file, output_path): file_err('The uploaded file contains inappropriate HTML content', dataset, json_file) return if data_type != 'binary': - if link_data_only == 'copy_files' and data_type not in ('gzip', 'bz2', 'zip'): + if not link_data_only and data_type not in ('gzip', 'bz2', 'zip'): # Convert universal line endings to Posix line endings if to_posix_lines is True # and the data is not binary or gzip-, bz2- or zip-compressed. if dataset.to_posix_lines: @@ -303,14 +303,14 @@ def add_file(dataset, registry, json_file, output_path): if ext == 'auto': ext = 'data' datatype = registry.get_datatype_by_extension(ext) - if dataset.type in ('server_dir', 'path_paste') and link_data_only == 'link_to_files': + if dataset.type in ('server_dir', 'path_paste') and link_data_only: # Never alter a file that will not be copied to Galaxy's local file store. if datatype.dataset_content_needs_grooming(dataset.path): err_msg = 'The uploaded files need grooming, so change your Copy data into Galaxy? selection to be ' + \ 'Copy files into Galaxy instead of Link to files without copying into Galaxy so grooming can be performed.' file_err(err_msg, dataset, json_file) return - if link_data_only == 'copy_files' and converted_path: + if not link_data_only and converted_path: # Move the dataset to its "real" path try: shutil.move(converted_path, output_path) @@ -318,7 +318,7 @@ def add_file(dataset, registry, json_file, output_path): # We may not have permission to remove converted_path if e.errno != errno.EACCES: raise - elif link_data_only == 'copy_files': + elif not link_data_only: if purge_source: shutil.move(dataset.path, output_path) else: @@ -334,7 +334,7 @@ def add_file(dataset, registry, json_file, output_path): if dataset.get('uuid', None) is not None: info['uuid'] = dataset.get('uuid') json_file.write(dumps(info) + "\n") - if link_data_only == 'copy_files' and datatype and datatype.dataset_content_needs_grooming(output_path): + if not link_data_only and datatype and datatype.dataset_content_needs_grooming(output_path): # Groom the dataset content if necessary datatype.groom_dataset_content(output_path) From 2528ef33cdd4f7b5b07c9de6e6adc5a35a7851b3 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 13 Dec 2017 09:21:53 -0500 Subject: [PATCH 3/4] Upload refactor - supply reasonable defaults to sniff methods making it easier to use. --- lib/galaxy/datatypes/sniff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 48ac2d46e6e..cf058cf069a 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -104,7 +104,7 @@ def check_newlines(fname, bytes_to_read=52428800): return False -def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix=None): +def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload"): """ Converts in place a file from universal line endings to Posix line endings. @@ -166,7 +166,7 @@ def sep2tabs(fname, in_place=True, patt="\\s+"): return (i, temp_name) -def convert_newlines_sep2tabs(fname, in_place=True, patt="\\s+", tmp_dir=None, tmp_prefix=None): +def convert_newlines_sep2tabs(fname, in_place=True, patt="\\s+", tmp_dir=None, tmp_prefix="gxupload"): """ Combines above methods: convert_newlines() and sep2tabs() so that files do not need to be read twice From 7e1bff7d69b0bc25056c6e2b18ccd2a689654c0e Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 15 Dec 2017 09:39:02 -0500 Subject: [PATCH 4/4] Upload refactor - change upload.py to use exceptions. Make decomposing and reuse of this easier downstream and feels cleaner to me. --- tools/data_source/upload.py | 68 ++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py index 0417d1944cb..8668df2afac 100644 --- a/tools/data_source/upload.py +++ b/tools/data_source/upload.py @@ -36,6 +36,12 @@ else: assert sys.version_info[:2] >= (2, 7) +class UploadProblemException(Exception): + + def __init__(self, message): + self.message = message + + def file_err(msg, dataset, json_file): json_file.write(dumps(dict(type='dataset', ext='data', @@ -104,24 +110,20 @@ def add_file(dataset, registry, json_file, output_path): try: ext = dataset.file_type except AttributeError: - file_err('Unable to process uploaded file, missing file_type parameter.', dataset, json_file) - return + raise UploadProblemException('Unable to process uploaded file, missing file_type parameter.') if dataset.type == 'url': try: page = urlopen(dataset.path) # page will be .close()ed by sniff methods temp_name = sniff.stream_to_file(page, prefix='url_paste', source_encoding=util.get_charset_from_http_headers(page.headers)) except Exception as e: - file_err('Unable to fetch %s\n%s' % (dataset.path, str(e)), dataset, json_file) - return + raise UploadProblemException('Unable to fetch %s\n%s' % (dataset.path, str(e))) dataset.path = temp_name # See if we have an empty file if not os.path.exists(dataset.path): - file_err('Uploaded temporary file (%s) does not exist.' % dataset.path, dataset, json_file) - return + raise UploadProblemException('Uploaded temporary file (%s) does not exist.' % dataset.path) if not os.path.getsize(dataset.path) > 0: - file_err('The uploaded file is empty', dataset, json_file) - return + raise UploadProblemException('The uploaded file is empty') # Is dataset content supported sniffable binary? is_binary = check_binary(dataset.path) if is_binary: @@ -141,8 +143,7 @@ def add_file(dataset, registry, json_file, output_path): # See if we have a gzipped file, which, if it passes our restrictions, we'll uncompress is_gzipped, is_valid = check_gzip(dataset.path, check_content=check_content) if is_gzipped and not is_valid: - file_err('The gzipped uploaded file contains inappropriate content', dataset, json_file) - return + raise UploadProblemException('The gzipped uploaded file contains inappropriate content') elif is_gzipped and is_valid and auto_decompress: if not link_data_only: # We need to uncompress the temp_name file, but BAM files must remain compressed in the BGZF format @@ -155,8 +156,7 @@ def add_file(dataset, registry, json_file, output_path): except IOError: os.close(fd) os.remove(uncompressed) - file_err('Problem decompressing gzipped data', dataset, json_file) - return + raise UploadProblemException('Problem decompressing gzipped data') if not chunk: break os.write(fd, chunk) @@ -174,8 +174,7 @@ def add_file(dataset, registry, json_file, output_path): # See if we have a bz2 file, much like gzip is_bzipped, is_valid = check_bz2(dataset.path, check_content) if is_bzipped and not is_valid: - file_err('The gzipped uploaded file contains inappropriate content', dataset, json_file) - return + raise UploadProblemException('The gzipped uploaded file contains inappropriate content') elif is_bzipped and is_valid and auto_decompress: if not link_data_only: # We need to uncompress the temp_name file @@ -188,8 +187,7 @@ def add_file(dataset, registry, json_file, output_path): except IOError: os.close(fd) os.remove(uncompressed) - file_err('Problem decompressing bz2 compressed data', dataset, json_file) - return + raise UploadProblemException('Problem decompressing bz2 compressed data') if not chunk: break os.write(fd, chunk) @@ -228,8 +226,7 @@ def add_file(dataset, registry, json_file, output_path): except IOError: os.close(fd) os.remove(uncompressed) - file_err('Problem decompressing zipped data', dataset, json_file) - return + raise UploadProblemException('Problem decompressing zipped data') if not chunk: break os.write(fd, chunk) @@ -247,8 +244,7 @@ def add_file(dataset, registry, json_file, output_path): except IOError: os.close(fd) os.remove(uncompressed) - file_err('Problem decompressing zipped data', dataset, json_file) - return + raise UploadProblemException('Problem decompressing zipped data') z.close() # Replace the zipped file with the decompressed file if it's safe to do so if uncompressed is not None: @@ -268,17 +264,14 @@ def add_file(dataset, registry, json_file, output_path): ext = parts[-1].strip().lower() is_ext_unsniffable_binary = registry.is_extension_unsniffable_binary(ext) if check_content and not is_ext_unsniffable_binary: - file_err('The uploaded binary file contains inappropriate content', dataset, json_file) - return + raise UploadProblemException('The uploaded binary file contains inappropriate content') elif is_ext_unsniffable_binary and dataset.file_type != ext: err_msg = "You must manually set the 'File Format' to '%s' when uploading %s files." % (ext, ext) - file_err(err_msg, dataset, json_file) - return + raise UploadProblemException(err_msg) if not data_type: # We must have a text file if check_content and check_html(dataset.path): - file_err('The uploaded file contains inappropriate HTML content', dataset, json_file) - return + raise UploadProblemException('The uploaded file contains inappropriate HTML content') if data_type != 'binary': if not link_data_only and data_type not in ('gzip', 'bz2', 'zip'): # Convert universal line endings to Posix line endings if to_posix_lines is True @@ -308,8 +301,7 @@ def add_file(dataset, registry, json_file, output_path): if datatype.dataset_content_needs_grooming(dataset.path): err_msg = 'The uploaded files need grooming, so change your Copy data into Galaxy? selection to be ' + \ 'Copy files into Galaxy instead of Link to files without copying into Galaxy so grooming can be performed.' - file_err(err_msg, dataset, json_file) - return + raise UploadProblemException(err_msg) if not link_data_only and converted_path: # Move the dataset to its "real" path try: @@ -345,8 +337,7 @@ def add_composite_file(dataset, json_file, output_path, files_path): for name, value in dataset.composite_files.items(): value = util.bunch.Bunch(**value) if dataset.composite_file_paths[value.name] is None and not value.optional: - file_err('A required composite data file was not provided (%s)' % name, dataset, json_file) - break + raise UploadProblemException('A required composite data file was not provided (%s)' % name) elif dataset.composite_file_paths[value.name] is not None: dp = dataset.composite_file_paths[value.name]['path'] isurl = dp.find('://') != -1 # todo fixme @@ -354,8 +345,7 @@ def add_composite_file(dataset, json_file, output_path, files_path): try: temp_name = sniff.stream_to_file(urlopen(dp), prefix='url_paste') except Exception as e: - file_err('Unable to fetch %s\n%s' % (dp, str(e)), dataset, json_file) - return + raise UploadProblemException('Unable to fetch %s\n%s' % (dp, str(e))) dataset.path = temp_name dp = temp_name if not value.is_binary: @@ -403,12 +393,14 @@ def __main__(): except Exception: print('Output path for dataset %s not found on command line' % dataset.dataset_id, file=sys.stderr) sys.exit(1) - if dataset.type == 'composite': - files_path = output_paths[int(dataset.dataset_id)][1] - add_composite_file(dataset, json_file, output_path, files_path) - else: - add_file(dataset, registry, json_file, output_path) - + try: + if dataset.type == 'composite': + files_path = output_paths[int(dataset.dataset_id)][1] + add_composite_file(dataset, json_file, output_path, files_path) + else: + add_file(dataset, registry, json_file, output_path) + except UploadProblemException as e: + file_err(e.message, dataset, json_file) # clean up paramfile # TODO: this will not work when running as the actual user unless the # parent directory is writable by the user.