From 32b85ecb591d01acbbbb9ed1bedd1a48804a0837 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 06:03:55 +0000
Subject: [PATCH 01/23] Only permit yaml.safe_loading of data

Event trusted data, belt + suspenders method.
---
 lib/galaxy/containers/__init__.py                           | 2 +-
 lib/galaxy/datatypes/registry.py                            | 2 +-
 lib/galaxy/jobs/runners/pulsar.py                           | 2 +-
 lib/galaxy/tools/deps/conda_compat.py                       | 2 +-
 lib/galaxy/tools/deps/resolvers/__init__.py                 | 2 +-
 lib/galaxy/tools/locations/dockstore.py                     | 2 +-
 lib/galaxy/tools/parser/factory.py                          | 2 +-
 lib/galaxy/tools/toolbox/parser.py                          | 2 +-
 lib/galaxy/tours/__init__.py                                | 2 +-
 lib/galaxy/util/plugin_config.py                            | 2 +-
 lib/galaxy/util/properties.py                               | 2 +-
 .../visualization/plugins/interactive_environments.py       | 2 +-
 lib/galaxy/webapps/config_manage.py                         | 4 ++--
 lib/galaxy/webapps/galaxy/api/users.py                      | 2 +-
 lib/galaxy/webapps/galaxy/controllers/visualization.py      | 2 +-
 lib/galaxy/webhooks/__init__.py                             | 2 +-
 scripts/grt/export.py                                       | 4 ++--
 scripts/grt/upload.py                                       | 4 ++--
 test/api/test_workflows.py                                  | 2 +-
 test/api/test_workflows_from_yaml.py                        | 6 +++---
 test/base/workflows_format_2/converter.py                   | 4 ++--
 test/base/workflows_format_2/main.py                        | 2 +-
 test/galaxy_selenium/data.py                                | 2 +-
 test/galaxy_selenium/navigates_galaxy.py                    | 2 +-
 test/unit/workflows/workflow_support.py                     | 2 +-
 25 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/lib/galaxy/containers/__init__.py b/lib/galaxy/containers/__init__.py
index 4b33dbe8d5a..6cebdd82e14 100644
--- a/lib/galaxy/containers/__init__.py
+++ b/lib/galaxy/containers/__init__.py
@@ -306,7 +306,7 @@ def parse_containers_config(containers_config_file):
     conf = DEFAULT_CONF.copy()
     try:
         with open(containers_config_file) as fh:
-            c = yaml.load(fh)
+            c = yaml.safe_load(fh)
             conf.update(c.get('containers', {}))
     except (OSError, IOError) as exc:
         if exc.errno == errno.ENOENT:
diff --git a/lib/galaxy/datatypes/registry.py b/lib/galaxy/datatypes/registry.py
index 31902b14513..f11b8498719 100644
--- a/lib/galaxy/datatypes/registry.py
+++ b/lib/galaxy/datatypes/registry.py
@@ -359,7 +359,7 @@ class Registry(object):
             build_sites_config_file = getattr(self.config, "build_sites_config_file", None)
             if build_sites_config_file and os.path.exists(build_sites_config_file):
                 with open(build_sites_config_file, "r") as f:
-                    build_sites_config = yaml.load(f)
+                    build_sites_config = yaml.safe_load(f)
                 if not isinstance(build_sites_config, list):
                     self.log.exception("Build sites configuration YAML file does not declare list of sites.")
                     return
diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py
index 9722ea32736..9fee6ef7f81 100644
--- a/lib/galaxy/jobs/runners/pulsar.py
+++ b/lib/galaxy/jobs/runners/pulsar.py
@@ -217,7 +217,7 @@ class PulsarJobRunner(AsynchronousJobRunner):
         else:
             log.info("Loading Pulsar app configuration from %s" % pulsar_conf_path)
             with open(pulsar_conf_path, "r") as f:
-                conf.update(yaml.load(f) or {})
+                conf.update(yaml.safe_load(f) or {})
         if "job_metrics_config_file" not in conf:
             conf["job_metrics"] = self.app.job_metrics
         if "staging_directory" not in conf:
diff --git a/lib/galaxy/tools/deps/conda_compat.py b/lib/galaxy/tools/deps/conda_compat.py
index c45573ef49d..10527bf890d 100644
--- a/lib/galaxy/tools/deps/conda_compat.py
+++ b/lib/galaxy/tools/deps/conda_compat.py
@@ -61,7 +61,7 @@ def _render_jinja2(recipe_dir):
 
 @_Memoized
 def yamlize(data):
-    res = yaml.load(data)
+    res = yaml.safe_load(data)
     # ensure the result is a dict
     if res is None:
         res = {}
diff --git a/lib/galaxy/tools/deps/resolvers/__init__.py b/lib/galaxy/tools/deps/resolvers/__init__.py
index e8188e0964a..5c3811024e0 100644
--- a/lib/galaxy/tools/deps/resolvers/__init__.py
+++ b/lib/galaxy/tools/deps/resolvers/__init__.py
@@ -97,7 +97,7 @@ class MappableDependencyResolver:
     @staticmethod
     def _mapping_file_to_list(mapping_file):
         with open(mapping_file, "r") as f:
-            raw_mapping = yaml.load(f) or []
+            raw_mapping = yaml.safe_load(f) or []
         return map(RequirementMapping.from_dict, raw_mapping)
 
     def _expand_mappings(self, requirement):
diff --git a/lib/galaxy/tools/locations/dockstore.py b/lib/galaxy/tools/locations/dockstore.py
index b379c23c38f..3eb96e55e24 100644
--- a/lib/galaxy/tools/locations/dockstore.py
+++ b/lib/galaxy/tools/locations/dockstore.py
@@ -57,7 +57,7 @@ class _Ga4ghToolClient(object):
         if as_string:
             return descriptor_str
         else:
-            return yaml.load(descriptor_str)
+            return yaml.safe_load(descriptor_str)
 
     @property
     def _requests(self):
diff --git a/lib/galaxy/tools/parser/factory.py b/lib/galaxy/tools/parser/factory.py
index 52a11ad7b57..b6655be63d5 100644
--- a/lib/galaxy/tools/parser/factory.py
+++ b/lib/galaxy/tools/parser/factory.py
@@ -62,7 +62,7 @@ def ordered_load(stream):
         yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG,
         construct_mapping)
 
-    return yaml.load(stream, OrderedLoader)
+    return yaml.safe_load(stream, OrderedLoader)
 
 
 def get_input_source(content):
diff --git a/lib/galaxy/tools/toolbox/parser.py b/lib/galaxy/tools/toolbox/parser.py
index 3c79a8b4d7b..db1b582404c 100644
--- a/lib/galaxy/tools/toolbox/parser.py
+++ b/lib/galaxy/tools/toolbox/parser.py
@@ -59,7 +59,7 @@ class YamlToolConfSource(ToolConfSource):
 
     def __init__(self, config_filename):
         with open(config_filename, "r") as f:
-            as_dict = yaml.load(f)
+            as_dict = yaml.safe_load(f)
         self.as_dict = as_dict
 
     def parse_tool_path(self):
diff --git a/lib/galaxy/tours/__init__.py b/lib/galaxy/tours/__init__.py
index 0511aa29e83..c35d147759c 100644
--- a/lib/galaxy/tours/__init__.py
+++ b/lib/galaxy/tours/__init__.py
@@ -68,7 +68,7 @@ class ToursRegistry(object):
         tour_id = os.path.splitext(filename)[0]
         try:
             with open(tour_path) as handle:
-                conf = yaml.load(handle)
+                conf = yaml.safe_load(handle)
                 tour = tour_loader(conf)
                 self.tours[tour_id] = tour_loader(conf)
                 log.info("Loaded tour '%s'" % tour_id)
diff --git a/lib/galaxy/util/plugin_config.py b/lib/galaxy/util/plugin_config.py
index 133998f210f..7f6866d6f2d 100644
--- a/lib/galaxy/util/plugin_config.py
+++ b/lib/galaxy/util/plugin_config.py
@@ -83,4 +83,4 @@ def __read_yaml(path):
         raise ImportError("Attempting to read YAML configuration file - but PyYAML dependency unavailable.")
 
     with open(path, "rb") as f:
-        return yaml.load(f)
+        return yaml.safe_load(f)
diff --git a/lib/galaxy/util/properties.py b/lib/galaxy/util/properties.py
index 2ca160c27f7..8c22e0aaf59 100644
--- a/lib/galaxy/util/properties.py
+++ b/lib/galaxy/util/properties.py
@@ -67,7 +67,7 @@ def load_app_properties(
                 config_section = "galaxy"
 
             with open(config_file, "r") as f:
-                raw_properties = yaml.load(f)
+                raw_properties = yaml.safe_load(f)
             properties = raw_properties[config_section] or {}
 
     override_prefix = "%sOVERRIDE_" % config_prefix
diff --git a/lib/galaxy/visualization/plugins/interactive_environments.py b/lib/galaxy/visualization/plugins/interactive_environments.py
index 161882e9409..602da1257e1 100644
--- a/lib/galaxy/visualization/plugins/interactive_environments.py
+++ b/lib/galaxy/visualization/plugins/interactive_environments.py
@@ -111,7 +111,7 @@ class InteractiveEnvironmentRequest(object):
                 raise Exception("[{0}] Could not find allowed_images.yml, or image tag in {0}.ini file for ".format(self.attr.viz_id))
 
         with open(fn, 'r') as handle:
-            self.allowed_images = [x['image'] for x in yaml.load(handle)]
+            self.allowed_images = [x['image'] for x in yaml.safe_load(handle)]
 
             if len(self.allowed_images) == 0:
                 raise Exception("No allowed images specified for " + self.attr.viz_id)
diff --git a/lib/galaxy/webapps/config_manage.py b/lib/galaxy/webapps/config_manage.py
index 07d0bc03658..4cad182179e 100644
--- a/lib/galaxy/webapps/config_manage.py
+++ b/lib/galaxy/webapps/config_manage.py
@@ -670,7 +670,7 @@ def _ordered_load(stream):
         def include(self, node):
             filename = os.path.join(self._root, self.construct_scalar(node))
             with open(filename, 'r') as f:
-                return yaml.load(f, OrderedLoader)
+                return yaml.safe_load(f, OrderedLoader)
 
     def construct_mapping(loader, node):
         loader.flatten_mapping(node)
@@ -681,7 +681,7 @@ def _ordered_load(stream):
         construct_mapping)
     OrderedLoader.add_constructor('!include', OrderedLoader.include)
 
-    return yaml.load(stream, OrderedLoader)
+    return yaml.safe_load(stream, OrderedLoader)
 
 
 def _ordered_dump(data, stream=None, Dumper=yaml.Dumper, **kwds):
diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py
index 67ec64351f3..63c9e3a78a0 100644
--- a/lib/galaxy/webapps/galaxy/api/users.py
+++ b/lib/galaxy/webapps/galaxy/api/users.py
@@ -282,7 +282,7 @@ class UserAPIController(BaseAPIController, UsesTagsMixin, CreatesUsersMixin, Cre
         path = trans.app.config.user_preferences_extra_config_file
         try:
             with open(path, 'r') as stream:
-                config = yaml.load(stream)
+                config = yaml.safe_load(stream)
         except:
             log.warning('Config file (%s) could not be found or is malformed.' % path)
             return {}
diff --git a/lib/galaxy/webapps/galaxy/controllers/visualization.py b/lib/galaxy/webapps/galaxy/controllers/visualization.py
index f134ebe48b4..9c7c1e5424e 100644
--- a/lib/galaxy/webapps/galaxy/controllers/visualization.py
+++ b/lib/galaxy/webapps/galaxy/controllers/visualization.py
@@ -961,7 +961,7 @@ class VisualizationController(BaseUIController, SharableMixin, UsesVisualization
                         continue
 
                     with open(image_file, 'r') as handle:
-                        self.gie_image_map[gie] = yaml.load(handle)
+                        self.gie_image_map[gie] = yaml.safe_load(handle)
 
         return trans.fill_template_mako(
             "visualization/gie.mako",
diff --git a/lib/galaxy/webhooks/__init__.py b/lib/galaxy/webhooks/__init__.py
index 7e9c9482433..6cc2c34d9d9 100644
--- a/lib/galaxy/webhooks/__init__.py
+++ b/lib/galaxy/webhooks/__init__.py
@@ -64,7 +64,7 @@ class WebhooksRegistry(object):
     def load_webhook_from_config(self, config_dir, config_file):
         try:
             with open(os.path.join(config_dir, config_file)) as file:
-                config = yaml.load(file)
+                config = yaml.safe_load(file)
                 path = os.path.normpath(os.path.join(config_dir, '..'))
                 webhook = Webhook(
                     config['name'],
diff --git a/scripts/grt/export.py b/scripts/grt/export.py
index 4619c6929c6..da8048522f2 100644
--- a/scripts/grt/export.py
+++ b/scripts/grt/export.py
@@ -191,11 +191,11 @@ def main(argv):
     annotate('init_start', 'Loading GRT configuration...')
     try:
         with open(args.config) as handle:
-            config = yaml.load(handle)
+            config = yaml.safe_load(handle)
     except Exception:
         logging.info('Using default GRT configuration')
         with open(sample_config) as handle:
-            config = yaml.load(handle)
+            config = yaml.safe_load(handle)
     annotate('init_end')
 
     REPORT_DIR = args.report_directory
diff --git a/scripts/grt/upload.py b/scripts/grt/upload.py
index 332cbe89e6d..a93efb86a62 100644
--- a/scripts/grt/upload.py
+++ b/scripts/grt/upload.py
@@ -28,11 +28,11 @@ def main(argv):
     logging.info('Loading GRT configuration...')
     try:
         with open(args.config) as handle:
-            config = yaml.load(handle)
+            config = yaml.safe_load(handle)
     except Exception:
         logging.info('Using default GRT configuration')
         with open(sample_config) as handle:
-            config = yaml.load(handle)
+            config = yaml.safe_load(handle)
 
     REPORT_DIR = args.report_directory
     GRT_URL = config['grt']['url'].rstrip('/') + '/'
diff --git a/test/api/test_workflows.py b/test/api/test_workflows.py
index 79a0c1c2218..f3c45c03368 100644
--- a/test/api/test_workflows.py
+++ b/test/api/test_workflows.py
@@ -172,7 +172,7 @@ class BaseWorkflowsApiTestCase(api.ApiTestCase):
         )
         if jobs_descriptions is None:
             assert source_type != "path"
-            jobs_descriptions = yaml.load(has_workflow)
+            jobs_descriptions = yaml.safe_load(has_workflow)
 
         test_data = jobs_descriptions.get("test_data", {})
 
diff --git a/test/api/test_workflows_from_yaml.py b/test/api/test_workflows_from_yaml.py
index 9beddf7aa2e..2b39628c27d 100644
--- a/test/api/test_workflows_from_yaml.py
+++ b/test/api/test_workflows_from_yaml.py
@@ -55,11 +55,11 @@ steps:
         assert tool_count['random_lines1'] == 1
         assert tool_count['cat1'] == 2
 
-# FIXME:  This test fails on some machines due to (we're guessing) yaml loading
+# FIXME:  This test fails on some machines due to (we're guessing) yaml.safe_loading
 # order being not guaranteed and inconsistent across platforms.  The workflow
-# yaml loader probably needs to enforce order using something like the
+# yaml.safe_loader probably needs to enforce order using something like the
 # approach described here:
-# https://stackoverflow.com/questions/13297744/pyyaml-control-ordering-of-items-called-by-yaml-load
+# https://stackoverflow.com/questions/13297744/pyyaml-control-ordering-of-items-called-by-yaml.safe_load
 #     def test_multiple_input( self ):
 #         history_id = self.dataset_populator.new_history()
 #         self._run_jobs("""
diff --git a/test/base/workflows_format_2/converter.py b/test/base/workflows_format_2/converter.py
index 05a9a319c49..c35ec2797c0 100644
--- a/test/base/workflows_format_2/converter.py
+++ b/test/base/workflows_format_2/converter.py
@@ -32,7 +32,7 @@ RUN_ACTIONS_TO_STEPS = {
 
 def yaml_to_workflow(has_yaml, galaxy_interface, workflow_directory):
     """Convert a Format 2 workflow into standard Galaxy format from supplied stream."""
-    as_python = yaml.load(has_yaml)
+    as_python = yaml.safe_load(has_yaml)
     return python_to_workflow(as_python, galaxy_interface, workflow_directory)
 
 
@@ -109,7 +109,7 @@ def _python_to_workflow(as_python, conversion_context):
                 run_action_path = run_action["@import"]
                 runnable_path = os.path.join(conversion_context.workflow_directory, run_action_path)
                 with open(runnable_path, "r") as f:
-                    runnable_description = yaml.load(f)
+                    runnable_description = yaml.safe_load(f)
                     run_action = runnable_description
 
             run_class = run_action["class"]
diff --git a/test/base/workflows_format_2/main.py b/test/base/workflows_format_2/main.py
index 70d59a8db8d..a4af30e8da6 100644
--- a/test/base/workflows_format_2/main.py
+++ b/test/base/workflows_format_2/main.py
@@ -20,7 +20,7 @@ def convert_and_import_workflow(has_workflow, **kwds):
         if workflow_directory is None:
             workflow_directory = os.path.dirname(has_workflow)
         with open(workflow_path, "r") as f:
-            has_workflow = yaml.load(f)
+            has_workflow = yaml.safe_load(f)
 
     if workflow_directory is not None:
         workflow_directory = os.path.abspath(workflow_directory)
diff --git a/test/galaxy_selenium/data.py b/test/galaxy_selenium/data.py
index a3d969f8a4f..ef15e27197b 100644
--- a/test/galaxy_selenium/data.py
+++ b/test/galaxy_selenium/data.py
@@ -3,4 +3,4 @@ from pkg_resources import resource_string
 import yaml
 
 data_yaml = resource_string(__name__, 'navigation-data.yml').decode("UTF-8")
-NAVIGATION_DATA = yaml.load(data_yaml)
+NAVIGATION_DATA = yaml.safe_load(data_yaml)
diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py
index cff486af9a7..1a60e7ada62 100644
--- a/test/galaxy_selenium/navigates_galaxy.py
+++ b/test/galaxy_selenium/navigates_galaxy.py
@@ -798,7 +798,7 @@ class NavigatesGalaxy(HasDriver):
         self.home()
 
         with open(path, "r") as f:
-            tour_dict = yaml.load(f)
+            tour_dict = yaml.safe_load(f)
         steps = tour_dict["steps"]
         for i, step in enumerate(steps):
             title = step.get("title", None)
diff --git a/test/unit/workflows/workflow_support.py b/test/unit/workflows/workflow_support.py
index ee04ded4fc9..6f4f567cb66 100644
--- a/test/unit/workflows/workflow_support.py
+++ b/test/unit/workflows/workflow_support.py
@@ -75,7 +75,7 @@ class TestToolbox(object):
 
 def yaml_to_model(has_dict, id_offset=100):
     if isinstance(has_dict, str):
-        has_dict = yaml.load(has_dict)
+        has_dict = yaml.safe_load(has_dict)
 
     workflow = model.Workflow()
     workflow.steps = []

From df41d4f4087f9b133750a2c350ea207753f30f54 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 06:17:27 +0000
Subject: [PATCH 02/23] Remove some shell=True and fix commands

---
 lib/galaxy/datatypes/converters/interval_to_coverage.py   | 5 +++--
 lib/galaxy/datatypes/converters/lped_to_pbed_converter.py | 6 +++---
 .../datatypes/converters/pbed_ldreduced_converter.py      | 3 +--
 lib/galaxy/datatypes/converters/pbed_to_lped_converter.py | 4 +---
 lib/galaxy/datatypes/text.py                              | 8 ++++----
 lib/galaxy/jobs/runners/util/job_script/__init__.py       | 2 +-
 lib/galaxy/jobs/runners/util/kill.py                      | 6 +++---
 lib/galaxy/jobs/transfer_manager.py                       | 8 ++++----
 8 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/lib/galaxy/datatypes/converters/interval_to_coverage.py b/lib/galaxy/datatypes/converters/interval_to_coverage.py
index b0b8bb4695c..d4ddd98927b 100644
--- a/lib/galaxy/datatypes/converters/interval_to_coverage.py
+++ b/lib/galaxy/datatypes/converters/interval_to_coverage.py
@@ -133,8 +133,9 @@ if __name__ == "__main__":
     # Sort through a tempfile first
     temp_file = tempfile.NamedTemporaryFile(mode="r")
     environ['LC_ALL'] = 'POSIX'
-    commandline = "sort -f -n -k %d -k %d -k %d -o %s %s" % (chr_col_1 + 1, start_col_1 + 1, end_col_1 + 1, temp_file.name, in_fname)
-    subprocess.check_call(commandline, shell=True)
+    subprocess.check_call([
+        'sort', '-f', '-n', '-k', chr_col_1 + 1, '-k', start_col_1 + 1, '-k', end_col_1 + 1, '-o', temp_file.name, in_fname
+    ])
 
     coverage = CoverageWriter(out_stream=open(out_fname, "a"),
                               chromCol=chr_col_2, positionCol=position_col_2,
diff --git a/lib/galaxy/datatypes/converters/lped_to_pbed_converter.py b/lib/galaxy/datatypes/converters/lped_to_pbed_converter.py
index 777259c3e0e..83b8050e46a 100644
--- a/lib/galaxy/datatypes/converters/lped_to_pbed_converter.py
+++ b/lib/galaxy/datatypes/converters/lped_to_pbed_converter.py
@@ -72,9 +72,9 @@ def rgConv(inpedfilepath, outhtmlname, outfilepath, plink):
     if not missval:
         print('### lped_to_pbed_converter.py cannot identify missing value in %s' % pedf)
         missval = '0'
-    cl = '%s --noweb --file %s --make-bed --out %s --missing-genotype %s' % (plink, inpedfilepath, outroot, missval)
-    p = subprocess.Popen(cl, shell=True, cwd=outfilepath)
-    p.wait()  # run plink
+    subprocess.check_call([plink, '--noweb', '--file', inpedfilepath,
+                           '--make-bed', '--out', outroot,
+                           '--missing-genotype', missval], cwd=outfilepath)
 
 
 def main():
diff --git a/lib/galaxy/datatypes/converters/pbed_ldreduced_converter.py b/lib/galaxy/datatypes/converters/pbed_ldreduced_converter.py
index 86c4a4d051f..fbe1834b8b1 100644
--- a/lib/galaxy/datatypes/converters/pbed_ldreduced_converter.py
+++ b/lib/galaxy/datatypes/converters/pbed_ldreduced_converter.py
@@ -41,8 +41,7 @@ def pruneLD(plinktasks=[], cd='./', vclbase=[]):
     for task in plinktasks:  # each is a list
         vcl = vclbase + task
         with open(plog, 'w') as sto:
-            x = subprocess.Popen(' '.join(vcl), shell=True, stdout=sto, stderr=sto, cwd=cd)
-            x.wait()
+            subprocess.check_call(vcl, stdout=sto, stderr=sto, cwd=cd)
         try:
             lplog = open(plog, 'r').readlines()
             lplog = [elem for elem in lplog if elem.find('Pruning SNP') == -1]
diff --git a/lib/galaxy/datatypes/converters/pbed_to_lped_converter.py b/lib/galaxy/datatypes/converters/pbed_to_lped_converter.py
index 023c6ee5f30..24a052384a3 100644
--- a/lib/galaxy/datatypes/converters/pbed_to_lped_converter.py
+++ b/lib/galaxy/datatypes/converters/pbed_to_lped_converter.py
@@ -40,9 +40,7 @@ def rgConv(inpedfilepath, outhtmlname, outfilepath, plink):
     """
     basename = os.path.split(inpedfilepath)[-1]  # get basename
     outroot = os.path.join(outfilepath, basename)
-    cl = '%s --noweb --bfile %s --recode --out %s ' % (plink, inpedfilepath, outroot)
-    p = subprocess.Popen(cl, shell=True, cwd=outfilepath)
-    p.wait()  # run plink
+    subprocess.check_call([plink, '--noweb', '--bfile', inpedfilepath, '--recode', '--out', outroot], cwd=outfilepath)
 
 
 def main():
diff --git a/lib/galaxy/datatypes/text.py b/lib/galaxy/datatypes/text.py
index 4d71302b3f0..c201b872b1f 100644
--- a/lib/galaxy/datatypes/text.py
+++ b/lib/galaxy/datatypes/text.py
@@ -148,11 +148,11 @@ class Ipynb(Json):
             ofilename = ofile_handle.name
             ofile_handle.close()
             try:
-                cmd = 'jupyter nbconvert --to html --template full %s --output %s' % (dataset.file_name, ofilename)
-                log.info("Calling command %s" % cmd)
-                subprocess.call(cmd, shell=True)
+                cmd = ['jupyter', 'nbconvert', '--to', 'html', '--template', 'full', dataset.file_name, '--output', ofilename]
+                log.info("Calling command %s", ' '.join(cmd))
+                subprocess.check_call(cmd)
                 ofilename = '%s.html' % ofilename
-            except:
+            except subprocess.CalledProcessError:
                 ofilename = dataset.file_name
                 log.exception('Command "%s" failed. Could not convert the Jupyter Notebook to HTML, defaulting to plain text.', cmd)
             return open(ofilename)
diff --git a/lib/galaxy/jobs/runners/util/job_script/__init__.py b/lib/galaxy/jobs/runners/util/job_script/__init__.py
index 77af249e5c4..4c4c6b86e45 100644
--- a/lib/galaxy/jobs/runners/util/job_script/__init__.py
+++ b/lib/galaxy/jobs/runners/util/job_script/__init__.py
@@ -119,7 +119,7 @@ def _handle_script_integrity(path, config):
     sleep_amt = getattr(config, "check_job_script_integrity_sleep", DEFAULT_INTEGRITY_SLEEP)
     for i in range(count):
         try:
-            proc = subprocess.Popen([path], shell=True, env={"ABC_TEST_JOB_SCRIPT_INTEGRITY_XYZ": "1"})
+            proc = subprocess.Popen([path], env={"ABC_TEST_JOB_SCRIPT_INTEGRITY_XYZ": "1"})
             proc.wait()
             if proc.returncode == 42:
                 script_integrity_verified = True
diff --git a/lib/galaxy/jobs/runners/util/kill.py b/lib/galaxy/jobs/runners/util/kill.py
index c0dbd913b13..1a458c63913 100644
--- a/lib/galaxy/jobs/runners/util/kill.py
+++ b/lib/galaxy/jobs/runners/util/kill.py
@@ -1,6 +1,6 @@
 import os
+import subprocess
 from platform import system
-from subprocess import Popen
 from time import sleep
 
 try:
@@ -41,8 +41,8 @@ def _stock_kill_pid(pid):
 
 def __kill_windows(pid):
     try:
-        Popen("taskkill /F /T /PID %i" % pid, shell=True)
-    except Exception:
+        subprocess.check_call(['taskkill', '/F', '/T', '/PID', pid])
+    except subprocess.CalledProcessError:
         pass
 
 
diff --git a/lib/galaxy/jobs/transfer_manager.py b/lib/galaxy/jobs/transfer_manager.py
index 01acd6aae8f..8f3cd4539ee 100644
--- a/lib/galaxy/jobs/transfer_manager.py
+++ b/lib/galaxy/jobs/transfer_manager.py
@@ -23,7 +23,7 @@ class TransferManager(object):
     def __init__(self, app):
         self.app = app
         self.sa_session = app.model.context.current
-        self.command = 'python %s' % os.path.abspath(os.path.join(os.getcwd(), 'scripts', 'transfer.py'))
+        self.command = ['python', os.path.abspath(os.path.join(os.getcwd(), 'scripts', 'transfer.py'))]
         if app.config.get_bool('enable_job_recovery', True):
             # Only one Galaxy server process should be able to recover jobs! (otherwise you'll have nasty race conditions)
             self.running = True
@@ -68,9 +68,9 @@ class TransferManager(object):
             # The transfer script should daemonize fairly quickly - if this is
             # not the case, this process will need to be moved to a
             # non-blocking method.
-            cmd = '%s %s' % (self.command, tj.id)
-            log.debug('Transfer command is: %s' % cmd)
-            p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+            cmd = self.command + [tj.id]
+            log.debug('Transfer command is: %s', ' '.join(cmd))
+            p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
             p.wait()
             output = p.stdout.read(32768)
             if p.returncode != 0:

From cd6a1afaa9980d958ad324fa987a07e7608bf841 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 06:26:33 +0000
Subject: [PATCH 03/23] Replace some os.system calls with subprocess

---
 lib/galaxy/datatypes/sequence.py  |  4 ++--
 lib/galaxy/datatypes/tabular.py   | 13 +++++--------
 lib/galaxy/jobs/runners/pulsar.py |  4 ++--
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/lib/galaxy/datatypes/sequence.py b/lib/galaxy/datatypes/sequence.py
index 1267ff3da4a..ee47ad70bbd 100644
--- a/lib/galaxy/datatypes/sequence.py
+++ b/lib/galaxy/datatypes/sequence.py
@@ -7,6 +7,7 @@ import logging
 import os
 import re
 import string
+import subprocess
 import sys
 from cgi import escape
 from itertools import islice
@@ -693,8 +694,7 @@ class BaseFastq (Sequence):
         else:
             commands = Sequence.get_split_commands_sequential(is_gzip(input_name), input_name, output_name, start_sequence, sequence_count)
         for cmd in commands:
-            if 0 != os.system(cmd):
-                raise Exception("Executing '%s' failed" % cmd)
+            subprocess.check_call(cmd, shell=True)
         return True
     process_split_file = staticmethod(process_split_file)
 
diff --git a/lib/galaxy/datatypes/tabular.py b/lib/galaxy/datatypes/tabular.py
index 0100448e65a..9a6f0eb2cb5 100644
--- a/lib/galaxy/datatypes/tabular.py
+++ b/lib/galaxy/datatypes/tabular.py
@@ -521,15 +521,12 @@ class Sam(Tabular):
         Multiple SAM files may each have headers. Since the headers should all be the same, remove
         the headers from files 1-n, keeping them in the first file only
         """
-        cmd = 'mv %s %s' % (split_files[0], output_file)
-        result = os.system(cmd)
-        if result != 0:
-            raise Exception('Result %s from %s' % (result, cmd))
+        shutil.move(split_files[0], output_file)
+
         if len(split_files) > 1:
-            cmd = 'egrep -v -h "^@" %s >> %s' % (' '.join(split_files[1:]), output_file)
-        result = os.system(cmd)
-        if result != 0:
-            raise Exception('Result %s from %s' % (result, cmd))
+            cmd = ['egrep', '-v', '-h' '^@'] + split_files[1:] + ['>>', output_file]
+            subprocess.check_call(cmd, shell=True)
+
     merge = staticmethod(merge)
 
     # Dataproviders
diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py
index 9fee6ef7f81..8c15475ee7c 100644
--- a/lib/galaxy/jobs/runners/pulsar.py
+++ b/lib/galaxy/jobs/runners/pulsar.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import  # Need to import pulsar_client absolutel
 import errno
 import logging
 import os
+import subprocess
 from distutils.version import LooseVersion
 from time import sleep
 
@@ -382,8 +383,7 @@ class PulsarJobRunner(AsynchronousJobRunner):
         prepare_input_files_cmds = getattr(job_wrapper, 'prepare_input_files_cmds', None)
         if prepare_input_files_cmds is not None:
             for cmd in prepare_input_files_cmds:  # run the commands to stage the input files
-                if 0 != os.system(cmd):
-                    raise Exception('Error running file staging command: %s' % cmd)
+                subprocess.check_call(cmd, shell=True)
             job_wrapper.prepare_input_files_cmds = None  # prevent them from being used in-line
 
     def _populate_parameter_defaults(self, job_destination):

From 73337725bdd654904563dc54b04083a23e4a252d Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 06:29:30 +0000
Subject: [PATCH 04/23] Two more commands to list

---
 lib/galaxy/objectstore/s3.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index f832688741e..0d8297316e8 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -68,7 +68,7 @@ class S3ObjectStore(ObjectStore):
             log.info("Cache cleaner manager started")
         # Test if 'axel' is available for parallel download and pull the key into cache
         try:
-            subprocess.call('axel')
+            subprocess.check_call(['axel'])
             self.use_axel = True
         except OSError:
             self.use_axel = False
@@ -341,7 +341,7 @@ class S3ObjectStore(ObjectStore):
                 log.debug("Parallel pulled key '%s' into cache to %s", rel_path, self._get_cache_path(rel_path))
                 ncores = multiprocessing.cpu_count()
                 url = key.generate_url(7200)
-                ret_code = subprocess.call("axel -a -n %s '%s'" % (ncores, url))
+                ret_code = subprocess.call(['axel', '-a', '-n', ncores, url])
                 if ret_code == 0:
                     return True
             else:

From c85db3fd68590d3d2f61b0b3f88026773d9563a0 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 06:57:24 +0000
Subject: [PATCH 05/23] Some more command refactoring

---
 lib/galaxy/datatypes/binary.py                | 37 ++++++++-----------
 lib/galaxy/tools/deps/mulled/mulled_build.py  | 15 +++-----
 .../tools/deps/mulled/mulled_build_channel.py |  3 +-
 3 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py
index b4ab524b778..0f8184cd1d1 100644
--- a/lib/galaxy/datatypes/binary.py
+++ b/lib/galaxy/datatypes/binary.py
@@ -253,18 +253,17 @@ class Bam(Binary):
             raise Exception(message)
 
         # Get the version of samtools via --version-only, if available
-        p = subprocess.Popen(['samtools', '--version-only'],
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE)
-        output, error = p.communicate()
-
-        # --version-only is available
-        # Format is <version x.y.z>+htslib-<a.b.c>
-        if p.returncode == 0:
+        try:
+            output = subprocess.check_output(['samtools', '--version-only'])
+            # --version-only is available
+            # Format is <version x.y.z>+htslib-<a.b.c>
             version = output.split('+')[0]
             return version
+        except subprocess.CalledProcessError:
+            # --version-only not available
+            pass
 
-        output = subprocess.Popen(['samtools'], stderr=subprocess.PIPE, stdout=subprocess.PIPE).communicate()[1]
+        output = subprocess.check_output(['samtools'])
         lines = output.split('\n')
         for line in lines:
             if line.lower().startswith('version'):
@@ -294,10 +293,8 @@ class Bam(Binary):
 
     def _is_coordinate_sorted(self, file_name):
         """See if the input BAM file is sorted from the header information."""
-        params = ["samtools", "view", "-H", file_name]
-        output = subprocess.Popen(params, stderr=subprocess.PIPE, stdout=subprocess.PIPE).communicate()[0]
-        # find returns -1 if string is not found
-        return output.find("SO:coordinate") != -1 or output.find("SO:sorted") != -1
+        output = subprocess.check_output(["samtools", "view", "-H", file_name])
+        return 'SO:coordinate' in output or 'SO:sorted' in output
 
     def dataset_content_needs_grooming(self, file_name):
         """See if file_name is a sorted BAM file"""
@@ -322,8 +319,7 @@ class Bam(Binary):
                 return False
             index_name = tempfile.NamedTemporaryFile(prefix="bam_index").name
             stderr_name = tempfile.NamedTemporaryFile(prefix="bam_index_stderr").name
-            command = 'samtools index %s %s' % (file_name, index_name)
-            proc = subprocess.Popen(args=command, shell=True, stderr=open(stderr_name, 'wb'))
+            proc = subprocess.Popen(['samtools', 'index', file_name, index_name], stderr=open(stderr_name, 'wb'))
             proc.wait()
             stderr = open(stderr_name).read().strip()
             if stderr:
@@ -366,8 +362,8 @@ class Bam(Binary):
         tmp_sorted_dataset_file_name_prefix = os.path.join(tmp_dir, 'sorted')
         stderr_name = tempfile.NamedTemporaryFile(dir=tmp_dir, prefix="bam_sort_stderr").name
         samtools_created_sorted_file_name = "%s.bam" % tmp_sorted_dataset_file_name_prefix  # samtools accepts a prefix, not a filename, it always adds .bam to the prefix
-        command = "samtools sort %s %s" % (file_name, tmp_sorted_dataset_file_name_prefix)
-        proc = subprocess.Popen(args=command, shell=True, cwd=tmp_dir, stderr=open(stderr_name, 'wb'))
+        proc = subprocess.Popen(['samtools', 'sort', file_name, tmp_sorted_dataset_file_name_prefix],
+                                cwd=tmp_dir, stderr=open(stderr_name, 'wb'))
         exit_code = proc.wait()
         # Did sort succeed?
         stderr = open(stderr_name).read().strip()
@@ -1309,11 +1305,8 @@ class ExcelXls(Binary):
     edam_format = "format_3468"
 
     def sniff(self, filename):
-        mime_type = subprocess.check_output("file --mime-type '{}'".format(filename), shell=True).rstrip()
-        if mime_type.find("application/vnd.ms-excel") != -1:
-            return True
-        else:
-            return False
+        mime_type = subprocess.check_output(['file', '--mime-type', filename]).strip()
+        return "application/vnd.ms-excel" in mime_type
 
     def get_mime(self):
         """Returns the mime type of the datatype"""
diff --git a/lib/galaxy/tools/deps/mulled/mulled_build.py b/lib/galaxy/tools/deps/mulled/mulled_build.py
index 5d362f12cd3..381afbe2718 100644
--- a/lib/galaxy/tools/deps/mulled/mulled_build.py
+++ b/lib/galaxy/tools/deps/mulled/mulled_build.py
@@ -116,17 +116,12 @@ def get_affected_packages(args):
     """
     recipes_dir = args.recipes_dir
     hours = args.diff_hours
-    cmd = """cd '%s' && git log --diff-filter=ACMRTUXB --name-only --pretty="" --since="%s hours ago" | grep -E '^recipes/.*/meta.yaml' | sort | uniq""" % (recipes_dir, hours)
-    pkg_list = check_output(cmd, shell=True)
-    ret = list()
-    for pkg in pkg_list.strip().split('\n'):
+    cmd = ['git', 'log', '--diff-filter=ACMRTUXB', '--name-only', '--pretty=""', '--since="%s hours ago"' % hours]
+    changed_files = subprocess.check_output(cmd, cwd=recipes_dir).strip().split('\n')
+    pkg_list = set([x for x in changed_files if x.startswith('recipes/') and x.endswith('meta.yaml')])
+    for pkg in pkg_list:
         if pkg and os.path.exists(os.path.join(recipes_dir, pkg)):
-            ret.append((get_pkg_name(args, pkg), get_tests(args, pkg)))
-    return ret
-
-
-def check_output(cmd, shell=True):
-    return subprocess.check_output(cmd, shell=shell)
+            yield (get_pkg_name(args, pkg), get_tests(args, pkg))
 
 
 def conda_versions(pkg_name, file_name):
diff --git a/lib/galaxy/tools/deps/mulled/mulled_build_channel.py b/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
index 23e3021187b..ff87189b65b 100644
--- a/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
+++ b/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
@@ -55,8 +55,7 @@ def _new_versions(quay, conda):
 
 def run_channel(args, build_last_n_versions=1):
     """Build list of involucro commands (as shell snippet) to run."""
-    pkgs = get_affected_packages(args)
-    for pkg_name, pkg_tests in pkgs:
+    for pkg_name, pkg_tests in get_affected_packages(args):
         repo_data = _fetch_repo_data(args)
         c = conda_versions(pkg_name, repo_data)
         # only package the most recent N versions

From 670897ec5ca2284c7fad453c3d595defe3cca280 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 07:03:48 +0000
Subject: [PATCH 06/23] One more os command

---
 lib/galaxy/webapps/reports/controllers/system.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/galaxy/webapps/reports/controllers/system.py b/lib/galaxy/webapps/reports/controllers/system.py
index 6359c1defd8..5c325c2239e 100644
--- a/lib/galaxy/webapps/reports/controllers/system.py
+++ b/lib/galaxy/webapps/reports/controllers/system.py
@@ -1,5 +1,6 @@
 import logging
 import os
+import subprocess
 from datetime import datetime, timedelta
 from decimal import Decimal
 
@@ -148,12 +149,11 @@ class System(BaseUIController):
                                    message=message)
 
     def get_disk_usage(self, file_path):
-        df_cmd = 'df -h ' + file_path
         is_sym_link = os.path.islink(file_path)
         file_system = disk_size = disk_used = disk_avail = disk_cap_pct = mount = None
-        df_file = os.popen(df_cmd)
-        while True:
-            df_line = df_file.readline()
+        df_output = subprocess.check_output(['df', '-h', file_path])
+
+        for df_line in df_output:
             df_line = df_line.strip()
             if df_line:
                 df_line = df_line.lower()
@@ -176,7 +176,6 @@ class System(BaseUIController):
                         pass
             else:
                 break  # EOF
-        df_file.close()
         return (file_system, disk_size, disk_used, disk_avail, disk_cap_pct, mount)
 
     @web.expose

From 9b8e76f9d9df0ae0ba6238d44a8f07b97fe34d6f Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 07:32:01 +0000
Subject: [PATCH 07/23] Convert urllib to requests

Which has built-in protection against urls like "file:///tmp/a"
---
 cron/build_chrom_db.py                        |  6 ++---
 cron/parse_builds.py                          |  5 ++---
 cron/parse_builds_3_sites.py                  |  7 +++---
 lib/galaxy/external_services/actions.py       |  4 ++--
 .../pacific_biosciences_smrt_portal.py        |  7 +++---
 lib/galaxy/managers/citations.py              |  8 +++----
 lib/galaxy/tools/data/__init__.py             |  5 ++---
 .../tools/imp_exp/unpack_tar_gz_archive.py    | 19 +++++++---------
 lib/galaxy/web/proxy/__init__.py              | 14 +++++-------
 lib/galaxy/webapps/config_manage.py           |  7 +++---
 .../webapps/galaxy/controllers/async.py       |  4 ++--
 .../galaxy/controllers/library_common.py      | 14 ++++++------
 lib/galaxy/webapps/galaxy/controllers/root.py |  6 ++---
 .../webapps/galaxy/controllers/workflow.py    |  4 ++--
 .../webapps/tool_shed/controllers/upload.py   | 12 +++++-----
 lib/tool_shed/capsule/capsule_manager.py      | 22 ++++++++-----------
 scripts/data_libraries/build_lucene_index.py  |  6 ++---
 scripts/edam_mapping.py                       |  4 ++--
 scripts/microbes/harvest_bacteria.py          |  6 ++---
 scripts/tool_shed/api/export.py               | 15 +++++--------
 scripts/transfer.py                           |  1 +
 21 files changed, 76 insertions(+), 100 deletions(-)

diff --git a/cron/build_chrom_db.py b/cron/build_chrom_db.py
index f6b2b8efc2f..d3cba98b8dc 100644
--- a/cron/build_chrom_db.py
+++ b/cron/build_chrom_db.py
@@ -15,10 +15,10 @@ from __future__ import print_function
 
 import fileinput
 import os
+import requests
 import sys
 
 from six.moves.urllib.parse import urlencode
-from six.moves.urllib.request import urlopen
 
 import parse_builds
 
@@ -36,8 +36,8 @@ def getchrominfo(url, db):
         "hgta_regionType": "",
         "position": "",
         "hgta_doTopSubmit": "get info"})
-    page = urlopen(URL)
-    for line in page:
+    page = requests.get(URL).text
+    for line in page.split('\n'):
         line = line.rstrip("\r\n")
         if line.startswith("#"):
             continue
diff --git a/cron/parse_builds.py b/cron/parse_builds.py
index c30197184f8..f61214186b7 100644
--- a/cron/parse_builds.py
+++ b/cron/parse_builds.py
@@ -6,21 +6,20 @@ build   description
 """
 from __future__ import print_function
 
+import requests
 import sys
 import xml.etree.ElementTree as ElementTree
 
-from six.moves.urllib.request import urlopen
 
 
 def getbuilds(url):
     try:
-        page = urlopen(url)
+        text = requests.get(url).text
     except:
         print("#Unable to open " + url)
         print("?\tunspecified (?)")
         sys.exit(1)
 
-    text = page.read()
     try:
         tree = ElementTree.fromstring(text)
     except:
diff --git a/cron/parse_builds_3_sites.py b/cron/parse_builds_3_sites.py
index b22cd449bf9..464064f4736 100644
--- a/cron/parse_builds_3_sites.py
+++ b/cron/parse_builds_3_sites.py
@@ -4,10 +4,9 @@ Connects to sites and determines which builds are available at each.
 """
 from __future__ import print_function
 
+import requests
 import xml.etree.ElementTree as ElementTree
 
-from six.moves.urllib.request import urlopen
-
 sites = ['http://genome.ucsc.edu/cgi-bin/',
          'http://archaea.ucsc.edu/cgi-bin/',
          'http://genome-test.cse.ucsc.edu/cgi-bin/']
@@ -20,11 +19,11 @@ def main():
         trackurl = sites[i] + "hgTracks?"
         builds = []
         try:
-            page = urlopen(site)
+            page = requests.get(site)
         except:
             print("#Unable to connect to " + site)
             continue
-        text = page.read()
+        text = page.text
         try:
             tree = ElementTree.fromstring(text)
         except:
diff --git a/lib/galaxy/external_services/actions.py b/lib/galaxy/external_services/actions.py
index 69fd20ca57b..71520265c49 100644
--- a/lib/galaxy/external_services/actions.py
+++ b/lib/galaxy/external_services/actions.py
@@ -1,6 +1,6 @@
 # Contains actions that are used in External Services
 import logging
-from urllib import urlopen
+import requests
 from galaxy.web import url_for
 from galaxy.util.template import fill_template
 from result_handlers.basic import ExternalServiceActionResultHandler
@@ -104,7 +104,7 @@ class ExternalServiceWebAPIActionResult(ExternalServiceResult):
     @property
     def content(self):
         if self._content is None:
-            self._content = urlopen(self.url).read()
+            self._content = requests.get(self.url).read()
         return self._content
 
 
diff --git a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
index a0e2707c951..267e0878655 100644
--- a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
+++ b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
@@ -4,10 +4,9 @@ produced by SMRT Portal.
 """
 import json
 import logging
+import requests
 from string import Template
 
-from six.moves.urllib.request import urlopen
-
 from .data_transfer import DataTransfer
 
 log = logging.getLogger(__name__)
@@ -88,8 +87,8 @@ class SMRTPortalPlugin(DataTransfer):
             if self._missing_params(job.params, ['smrt_host', 'smrt_job_id']):
                 return self.job_states.INVALID
             url = 'http://' + job.params['smrt_host'] + self.api_path + '/Jobs/' + job.params['smrt_job_id'] + '/Status'
-            r = urlopen(url)
-            status = json.loads(r.read())
+            r = requests.get(url)
+            status = r.json()
             # TODO: error handling: unexpected json or bad response, bad url, etc.
             if status['Code'] == 'Completed':
                 log.debug("SMRT Portal job '%s' is Completed.  Initiating transfer." % job.params['smrt_job_id'])
diff --git a/lib/galaxy/managers/citations.py b/lib/galaxy/managers/citations.py
index 7b88b2a3728..5ff70e7c666 100644
--- a/lib/galaxy/managers/citations.py
+++ b/lib/galaxy/managers/citations.py
@@ -1,6 +1,6 @@
 import functools
 import os
-import urllib2
+import requests
 
 from beaker.cache import CacheManager
 from beaker.util import parse_cache_config_options
@@ -47,10 +47,8 @@ class DoiCache(object):
     def _raw_get_bibtex(self, doi):
         dx_url = "http://dx.doi.org/" + doi
         headers = {'Accept': 'text/bibliography; style=bibtex, application/x-bibtex'}
-        req = urllib2.Request(dx_url, data="", headers=headers)
-        response = urllib2.urlopen(req)
-        bibtex = response.read()
-        return bibtex
+        req = requests.get(dx_url, headers=headers)
+        return req.text
 
     def get_bibtex(self, doi):
         createfunc = functools.partial(self._raw_get_bibtex, doi)
diff --git a/lib/galaxy/tools/data/__init__.py b/lib/galaxy/tools/data/__init__.py
index 92aa6c995ed..98013c5201e 100644
--- a/lib/galaxy/tools/data/__init__.py
+++ b/lib/galaxy/tools/data/__init__.py
@@ -11,13 +11,12 @@ import logging
 import os
 import os.path
 import re
+import requests
 import string
 import time
 from glob import glob
 from tempfile import NamedTemporaryFile
 
-from six.moves.urllib.request import urlopen
-
 from galaxy import util
 from galaxy.util.dictifiable import Dictifiable
 from galaxy.util.odict import odict
@@ -340,7 +339,7 @@ class TabularToolDataTable(ToolDataTable, Dictifiable):
                 if filename:
                     tmp_file = NamedTemporaryFile(prefix='TTDT_URL_%s-' % self.name)
                     try:
-                        tmp_file.write(urlopen(filename, timeout=url_timeout).read())
+                        tmp_file.write(requests.get(filename, timeout=url_timeout).text)
                     except Exception as e:
                         log.error('Error loading Data Table URL "%s": %s', filename, e)
                         continue
diff --git a/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py b/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py
index c00a636d5a7..31e5c922a6d 100644
--- a/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py
+++ b/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py
@@ -10,13 +10,12 @@ from __future__ import print_function
 import math
 import optparse
 import os
+import requests
 import sys
 import tarfile
 import tempfile
 from base64 import b64decode
 
-from six.moves.urllib.request import urlopen
-
 # Set max size of archive/file that will be handled to be 100 GB. This is
 # arbitrary and should be adjusted as needed.
 MAX_SIZE = 100 * math.pow(2, 30)
@@ -27,18 +26,16 @@ def url_to_file(url, dest_file):
     Transfer a file from a remote URL to a temporary file.
     """
     try:
-        url_reader = urlopen(url)
+        url_reader = requests.get(url, stream=True)
         CHUNK = 10 * 1024  # 10k
         total = 0
         fp = open(dest_file, 'wb')
-        while True:
-            chunk = url_reader.read(CHUNK)
-            if not chunk:
-                break
-            fp.write(chunk)
-            total += CHUNK
-            if total > MAX_SIZE:
-                break
+        for chunk in url_reader.iter_content(chunk_size=CHUNK):
+            if chunk:
+                fp.write(chunk)
+                total += CHUNK
+                if total > MAX_SIZE:
+                    break
         fp.close()
         return dest_file
     except Exception as e:
diff --git a/lib/galaxy/web/proxy/__init__.py b/lib/galaxy/web/proxy/__init__.py
index 6ff3ee3399f..fcb23ba56c0 100644
--- a/lib/galaxy/web/proxy/__init__.py
+++ b/lib/galaxy/web/proxy/__init__.py
@@ -8,7 +8,7 @@ from galaxy.util import sockets
 from galaxy.util.lazy_process import LazyProcess, NoOpLazyProcess
 from galaxy.util import sqlite
 from galaxy.util import unique_id
-import urllib2
+import requests
 import time
 
 log = logging.getLogger(__name__)
@@ -301,21 +301,17 @@ class RestGolangProxyIpc(object):
             'ContainerIds': container_ids,
         }
 
-        req = urllib2.Request(self.api_url)
-        req.add_header('Content-Type', 'application/json')
-
         # Sometimes it takes our poor little proxy a second or two to get
         # going, so if this fails, re-call ourselves with an increased timeout.
         try:
-            urllib2.urlopen(req, json.dumps(values))
-        except urllib2.URLError as err:
-            log.debug(err)
+            requests.get(self.api_url, headers={'Content-Type': 'application/json'}, data=json.dumps(values))
+        except requests.exceptions.ConnectionError as err:
+            log.exception(err)
             if sleep > 5:
                 excp = "Could not contact proxy after %s seconds" % sum(range(sleep + 1))
                 raise Exception(excp)
             time.sleep(sleep)
-            self.handle_requests(authentication, proxy_requests, route_name, container_ids, sleep=sleep + 1)
-        pass
+            self.handle_requests(authentication, proxy_requests, route_name, container_ids, container_interface, sleep=sleep + 1)
 
 
 ProxyMapping = namedtuple('ProxyMapping', ['host', 'port', 'container_ids', 'container_interface'])
diff --git a/lib/galaxy/webapps/config_manage.py b/lib/galaxy/webapps/config_manage.py
index 4cad182179e..306cd7605f2 100644
--- a/lib/galaxy/webapps/config_manage.py
+++ b/lib/galaxy/webapps/config_manage.py
@@ -11,7 +11,7 @@ import string
 import sys
 import tempfile
 from textwrap import TextWrapper
-import urllib2
+import requests
 
 import six
 from six import StringIO
@@ -307,9 +307,8 @@ def _write_option_rst(args, rst, key, heading_level, option_value):
 
 
 def _build_uwsgi_schema(args, app_desc):
-    req = urllib2.Request('https://raw.githubusercontent.com/unbit/uwsgi-docs/master/Options.rst')
-    response = urllib2.urlopen(req)
-    rst_options = response.read()
+    req = requests.get('https://raw.githubusercontent.com/unbit/uwsgi-docs/master/Options.rst')
+    rst_options = req.text
     last_line = None
     current_opt = None
 
diff --git a/lib/galaxy/webapps/galaxy/controllers/async.py b/lib/galaxy/webapps/galaxy/controllers/async.py
index 7db4af7667f..001e832aea5 100644
--- a/lib/galaxy/webapps/galaxy/controllers/async.py
+++ b/lib/galaxy/webapps/galaxy/controllers/async.py
@@ -4,6 +4,7 @@ Upload class
 
 import logging
 import urllib
+import requests
 
 from galaxy import jobs, web
 from galaxy.util import Params
@@ -162,8 +163,7 @@ class ASync(BaseUIController):
                 url = "%s%s%s" % (url, url_join_char, urllib.urlencode(params.flatten()))
                 log.debug("connecting to -> %s" % url)
                 trans.log_event("Async connecting to -> %s" % url)
-                text = urllib.urlopen(url).read(-1)
-                text = text.strip()
+                text = requests.get(url).text.strip()
                 if not text.endswith('OK'):
                     raise Exception(text)
                 data.state = data.blurb = data.states.RUNNING
diff --git a/lib/galaxy/webapps/galaxy/controllers/library_common.py b/lib/galaxy/webapps/galaxy/controllers/library_common.py
index 365dd48c6b9..a35d6a261ef 100644
--- a/lib/galaxy/webapps/galaxy/controllers/library_common.py
+++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py
@@ -1,16 +1,16 @@
 import glob
+import json
 import logging
 import operator
 import os
 import os.path
+import requests
 import string
 import sys
 import tarfile
 import tempfile
 import urllib
-import urllib2
 import zipfile
-from json import dumps, loads
 
 from markupsafe import escape
 from sqlalchemy import and_, false
@@ -555,7 +555,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
             if len(em_string):
                 payload = None
                 try:
-                    payload = loads(em_string)
+                    payload = json.loads(em_string)
                 except Exception:
                     message = 'Invalid JSON input'
                     status = 'error'
@@ -1122,8 +1122,8 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMeta
         json_file_path = upload_common.create_paramfile(trans, uploaded_datasets)
         data_list = [ud.data for ud in uploaded_datasets]
         job_params = {}
-        job_params['link_data_only'] = dumps(kwd.get('link_data_only', 'copy_files'))
-        job_params['uuid'] = dumps(kwd.get('uuid', None))
+        job_params['link_data_only'] = json.dumps(kwd.get('link_data_only', 'copy_files'))
+        job_params['uuid'] = json.dumps(kwd.get('uuid', None))
         job, output = upload_common.create_job(trans, tool_params, tool, json_file_path, data_list, folder=library_bunch.folder, job_params=job_params)
         trans.sa_session.add(job)
         trans.sa_session.flush()
@@ -2762,8 +2762,8 @@ def lucene_search(trans, cntrller, search_term, search_url, **kwd):
     message = escape(kwd.get('message', ''))
     status = kwd.get('status', 'done')
     full_url = "%s/find?%s" % (search_url, urllib.urlencode({"kwd" : search_term}))
-    response = urllib2.urlopen(full_url)
-    ldda_ids = loads(response.read())["ids"]
+    response = requests.get(full_url).text
+    ldda_ids = json.loads(response)["ids"]
     response.close()
     lddas = [trans.sa_session.query(trans.app.model.LibraryDatasetDatasetAssociation).get(ldda_id) for ldda_id in ldda_ids]
     return status, message, get_sorted_accessible_library_items(trans, cntrller, lddas, 'name')
diff --git a/lib/galaxy/webapps/galaxy/controllers/root.py b/lib/galaxy/webapps/galaxy/controllers/root.py
index 4817aee5b84..6913f705080 100644
--- a/lib/galaxy/webapps/galaxy/controllers/root.py
+++ b/lib/galaxy/webapps/galaxy/controllers/root.py
@@ -3,7 +3,7 @@ Contains the main interface in the Universe class
 """
 import cgi
 import os
-import urllib
+import requests
 
 from paste.httpexceptions import HTTPNotFound, HTTPBadGateway
 
@@ -470,8 +470,8 @@ class RootController(controller.JSAppLauncher, UsesAnnotations):
     def bucket_proxy(self, trans, bucket=None, **kwd):
         if bucket:
             trans.response.set_content_type('text/xml')
-            b_list_xml = urllib.urlopen('http://s3.amazonaws.com/%s/' % bucket)
-            return b_list_xml.read()
+            b_list_xml = requests.get('http://s3.amazonaws.com/%s/' % bucket)
+            return b_list_xml.text
         raise Exception("You must specify a bucket")
 
     # ---- Debug methods ----------------------------------------------------
diff --git a/lib/galaxy/webapps/galaxy/controllers/workflow.py b/lib/galaxy/webapps/galaxy/controllers/workflow.py
index d5216798bed..55706557e22 100644
--- a/lib/galaxy/webapps/galaxy/controllers/workflow.py
+++ b/lib/galaxy/webapps/galaxy/controllers/workflow.py
@@ -4,7 +4,7 @@ import json
 import logging
 import os
 import sgmllib
-import urllib2
+import requests
 
 from sqlalchemy import and_
 from sqlalchemy.orm import joinedload
@@ -763,7 +763,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi
                 # Load workflow from external URL
                 # NOTE: blocks the web thread.
                 try:
-                    workflow_data = urllib2.urlopen(url).read()
+                    workflow_data = requests.get(url).text
                 except Exception as e:
                     message = "Failed to open URL: %s. Exception: %s" % (escape(url), escape(str(e)))
                     status = 'error'
diff --git a/lib/galaxy/webapps/tool_shed/controllers/upload.py b/lib/galaxy/webapps/tool_shed/controllers/upload.py
index 1695cf69507..b4c4dccd8d5 100644
--- a/lib/galaxy/webapps/tool_shed/controllers/upload.py
+++ b/lib/galaxy/webapps/tool_shed/controllers/upload.py
@@ -1,9 +1,9 @@
 import logging
 import os
+import requests
 import shutil
 import tarfile
 import tempfile
-import urllib
 
 from galaxy import util
 from galaxy import web
@@ -74,7 +74,7 @@ class UploadController(BaseUIController):
             elif url:
                 valid_url = True
                 try:
-                    stream = urllib.urlopen(url)
+                    stream = requests.get(url, stream=True)
                 except Exception as e:
                     valid_url = False
                     message = 'Error uploading file via http: %s' % str(e)
@@ -83,11 +83,9 @@ class UploadController(BaseUIController):
                 if valid_url:
                     fd, uploaded_file_name = tempfile.mkstemp()
                     uploaded_file = open(uploaded_file_name, 'wb')
-                    while 1:
-                        chunk = stream.read(util.CHUNK_SIZE)
-                        if not chunk:
-                            break
-                        uploaded_file.write(chunk)
+                    for chunk in stream.iter_content(chunk_size=util.CHUNK_SIZE):
+                        if chunk:
+                            uploaded_file.write(chunk)
                     uploaded_file.flush()
                     uploaded_file_filename = url.split('/')[-1]
                     isempty = os.path.getsize(os.path.abspath(uploaded_file_name)) == 0
diff --git a/lib/tool_shed/capsule/capsule_manager.py b/lib/tool_shed/capsule/capsule_manager.py
index 422d663c4a9..dcd192ce2f4 100644
--- a/lib/tool_shed/capsule/capsule_manager.py
+++ b/lib/tool_shed/capsule/capsule_manager.py
@@ -1,13 +1,13 @@
 import contextlib
 import logging
 import os
+import requests
 import shutil
 import tarfile
 import tempfile
 import threading
 from time import gmtime, strftime
 
-from six.moves.urllib.request import urlopen
 from sqlalchemy import and_, false
 
 import tool_shed.repository_types.util as rt_util
@@ -809,24 +809,20 @@ class ImportRepositoryManager(object):
                            uploaded_file=None,
                            capsule_file_name=None)
         if url:
-            valid_url = True
             try:
-                stream = urlopen(url)
+                stream = requests.get(url, stream=True)
             except Exception as e:
-                valid_url = False
                 return_dict['error_message'] = 'Error importing file via http: %s' % str(e)
                 return_dict['status'] = 'error'
                 return return_dict
-            if valid_url:
-                fd, uploaded_file_name = tempfile.mkstemp()
-                uploaded_file = open(uploaded_file_name, 'wb')
-                while 1:
-                    chunk = stream.read(CHUNK_SIZE)
-                    if not chunk:
-                        break
+
+            fd, uploaded_file_name = tempfile.mkstemp()
+            uploaded_file = open(uploaded_file_name, 'wb')
+            for chunk in stream.iter_content(chunk_size=CHUNK_SIZE):
+                if chunk:
                     uploaded_file.write(chunk)
-                uploaded_file.flush()
-                uploaded_file_filename = url.split('/')[-1]
+            uploaded_file.flush()
+            uploaded_file_filename = url.split('/')[-1]
         elif file_data not in ('', None):
             uploaded_file = file_data.file
             uploaded_file_name = uploaded_file.name
diff --git a/scripts/data_libraries/build_lucene_index.py b/scripts/data_libraries/build_lucene_index.py
index a53ec522163..710747068be 100644
--- a/scripts/data_libraries/build_lucene_index.py
+++ b/scripts/data_libraries/build_lucene_index.py
@@ -14,7 +14,7 @@ import csv
 import os
 import sys
 import urllib
-import urllib2
+import requests
 
 sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, 'lib')))
 
@@ -39,9 +39,7 @@ def main(ini_file):
 
 def build_index(search_url, dataset_file):
     url = "%s/index?%s" % (search_url, urllib.urlencode({"docfile": dataset_file}))
-    request = urllib2.Request(url)
-    request.get_method = lambda: "PUT"
-    urllib2.urlopen(request)
+    requests.put(url)
 
 
 def create_dataset_file(dataset_iter):
diff --git a/scripts/edam_mapping.py b/scripts/edam_mapping.py
index 7858d9f5abb..7e70c5467cd 100644
--- a/scripts/edam_mapping.py
+++ b/scripts/edam_mapping.py
@@ -16,7 +16,7 @@ from __future__ import print_function
 
 import os
 import sys
-import urllib2
+import requests
 from xml import etree
 
 sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib')))
@@ -35,7 +35,7 @@ EDAM_OWL_URL = "http://data.bioontology.org/ontologies/EDAM/submissions/25/downl
 
 
 if not os.path.exists("/tmp/edam.owl"):
-    open("/tmp/edam.owl", "w").write(urllib2.urlopen(EDAM_OWL_URL).read())
+    open("/tmp/edam.owl", "w").write(requests.get(EDAM_OWL_URL).text)
 
 
 owl_xml_tree = etree.ElementTree.parse("/tmp/edam.owl")
diff --git a/scripts/microbes/harvest_bacteria.py b/scripts/microbes/harvest_bacteria.py
index 7161669b5c9..175ce70cb0d 100644
--- a/scripts/microbes/harvest_bacteria.py
+++ b/scripts/microbes/harvest_bacteria.py
@@ -8,7 +8,7 @@ import os
 import sys
 import time
 from ftplib import FTP
-from urllib2 import urlopen
+import requests
 from urllib import urlretrieve
 
 from BeautifulSoup import BeautifulSoup
@@ -26,7 +26,7 @@ desired_ftp_files = {'GeneMark': {'ext': 'GeneMark-2.5f', 'parser': 'process_Gen
 
 # number, name, chroms, kingdom, group, genbank, refseq, info_url, ftp_url
 def iter_genome_projects(url="http://www.ncbi.nlm.nih.gov/genomes/lproks.cgi?view=1", info_url_base="http://www.ncbi.nlm.nih.gov/entrez/query.fcgi?db=genomeprj&cmd=Retrieve&dopt=Overview&list_uids="):
-    for row in BeautifulSoup(urlopen(url)).findAll(name='tr', bgcolor=["#EEFFDD", "#E8E8DD"]):
+    for row in BeautifulSoup(requests.get(url).text).findAll(name='tr', bgcolor=["#EEFFDD", "#E8E8DD"]):
         row = str(row).replace("\n", "").replace("\r", "")
 
         fields = row.split("</td>")
@@ -65,7 +65,7 @@ def get_chroms_by_project_id(org_num, base_url="http://www.ncbi.nlm.nih.gov/entr
         html_count += 1
         url = "%s%s" % (base_url, org_num)
         try:
-            html = urlopen(url)
+            html = requests.get(url).text
         except:
             print "GENOME PROJECT FAILED:", html_count, "org:", org_num, url
             html = None
diff --git a/scripts/tool_shed/api/export.py b/scripts/tool_shed/api/export.py
index a198fffd966..68005de0746 100644
--- a/scripts/tool_shed/api/export.py
+++ b/scripts/tool_shed/api/export.py
@@ -11,7 +11,7 @@ import argparse
 import os
 import sys
 import tempfile
-import urllib2
+import requests
 
 sys.path.insert(1, os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, os.pardir, 'lib'))
 from tool_shed.util import basic_util
@@ -101,14 +101,11 @@ def main(options):
             src = None
             dst = None
             try:
-                src = urllib2.urlopen(download_url)
-                dst = open(file_path, 'wb')
-                while True:
-                    chunk = src.read(CHUNK_SIZE)
-                    if chunk:
-                        dst.write(chunk)
-                    else:
-                        break
+                src = requests.get(download_url, stream=True)
+                with open(file_path, 'wb') as handle:
+                    for chunk in src.iter_content(chunk_size=CHUNK_SIZE):
+                        if chunk:
+                            handle.write(chunk)
             except:
                 raise
             finally:
diff --git a/scripts/transfer.py b/scripts/transfer.py
index 8bcb23f432e..3938ca48846 100644
--- a/scripts/transfer.py
+++ b/scripts/transfer.py
@@ -215,6 +215,7 @@ def transfer(app, transfer_job_id):
 def http_transfer(transfer_job):
     """Plugin" for handling http(s) transfers."""
     url = transfer_job.params['url']
+    assert url.startswith('http://') or url.startswith('https://')
     try:
         f = urllib2.urlopen(url)
     except urllib2.URLError as e:

From db0400298651f6d4c9d909406c3fe632d057bbf7 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 07:47:54 +0000
Subject: [PATCH 08/23] remove unused imports

---
 cron/parse_builds.py                                        | 1 -
 lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py | 1 -
 2 files changed, 2 deletions(-)

diff --git a/cron/parse_builds.py b/cron/parse_builds.py
index f61214186b7..e728315be13 100644
--- a/cron/parse_builds.py
+++ b/cron/parse_builds.py
@@ -11,7 +11,6 @@ import sys
 import xml.etree.ElementTree as ElementTree
 
 
-
 def getbuilds(url):
     try:
         text = requests.get(url).text
diff --git a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
index 267e0878655..0adf41e5e02 100644
--- a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
+++ b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
@@ -2,7 +2,6 @@
 Module for managing jobs in Pacific Bioscience's SMRT Portal and automatically transferring files
 produced by SMRT Portal.
 """
-import json
 import logging
 import requests
 from string import Template

From a68d6be3a1c6e6fd85c5ae94232467f193156d53 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 07:52:48 +0000
Subject: [PATCH 09/23] reorder imports

---
 test/functional/webhooks/phdcomics/helper/__init__.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/functional/webhooks/phdcomics/helper/__init__.py b/test/functional/webhooks/phdcomics/helper/__init__.py
index b6828d2c21a..d3ec4d7fbe5 100644
--- a/test/functional/webhooks/phdcomics/helper/__init__.py
+++ b/test/functional/webhooks/phdcomics/helper/__init__.py
@@ -1,7 +1,7 @@
-import urllib
-import re
-import random
 import logging
+import random
+import re
+import urllib
 
 log = logging.getLogger(__name__)
 

From 93ffc4055b145811b86191dae8b865ea45b55fcb Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 08:05:06 +0000
Subject: [PATCH 10/23] replace check_output with python processing and
 shell=False

---
 lib/galaxy/tools/deps/container_resolvers/mulled.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/tools/deps/container_resolvers/mulled.py b/lib/galaxy/tools/deps/container_resolvers/mulled.py
index fc551cb883a..cf215a8f742 100644
--- a/lib/galaxy/tools/deps/container_resolvers/mulled.py
+++ b/lib/galaxy/tools/deps/container_resolvers/mulled.py
@@ -3,6 +3,7 @@
 import collections
 import logging
 import os
+import subprocess
 
 import six
 
@@ -11,7 +12,6 @@ from ..container_resolvers import (
 )
 from ..docker_util import build_docker_images_command
 from ..mulled.mulled_build import (
-    check_output,
     DEFAULT_CHANNELS,
     ensure_installed,
     InvolucroContext,
@@ -54,8 +54,8 @@ CachedV2MulledImageMultiTarget.package_hash = _package_hash
 
 def list_docker_cached_mulled_images(namespace=None, hash_func="v2"):
     command = build_docker_images_command(truncate=True, sudo=False)
-    command = "%s | tail -n +2 | tr -s ' ' | cut -d' ' -f1,2" % command
-    images_and_versions = check_output(command)
+    images_and_versions = subprocess.check_output(command).strip().split('\n')
+    images_and_versions = [line.split()[0:2] for line in images_and_versions[1:]]
     name_filter = get_filter(namespace)
 
     def output_line_to_image(line):

From d79a2459a95dfda7856b9898d40d062f00456006 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 11:28:58 +0200
Subject: [PATCH 11/23] These we will revert to their original yaml.load

It would be nice to fix them eventually, but we'll settle for an
incremental improvement.
---
 lib/galaxy/tools/parser/factory.py  | 2 +-
 lib/galaxy/webapps/config_manage.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/tools/parser/factory.py b/lib/galaxy/tools/parser/factory.py
index b6655be63d5..52a11ad7b57 100644
--- a/lib/galaxy/tools/parser/factory.py
+++ b/lib/galaxy/tools/parser/factory.py
@@ -62,7 +62,7 @@ def ordered_load(stream):
         yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG,
         construct_mapping)
 
-    return yaml.safe_load(stream, OrderedLoader)
+    return yaml.load(stream, OrderedLoader)
 
 
 def get_input_source(content):
diff --git a/lib/galaxy/webapps/config_manage.py b/lib/galaxy/webapps/config_manage.py
index 306cd7605f2..ee7b6b38069 100644
--- a/lib/galaxy/webapps/config_manage.py
+++ b/lib/galaxy/webapps/config_manage.py
@@ -669,7 +669,7 @@ def _ordered_load(stream):
         def include(self, node):
             filename = os.path.join(self._root, self.construct_scalar(node))
             with open(filename, 'r') as f:
-                return yaml.safe_load(f, OrderedLoader)
+                return yaml.load(f, OrderedLoader)
 
     def construct_mapping(loader, node):
         loader.flatten_mapping(node)
@@ -680,7 +680,7 @@ def _ordered_load(stream):
         construct_mapping)
     OrderedLoader.add_constructor('!include', OrderedLoader.include)
 
-    return yaml.safe_load(stream, OrderedLoader)
+    return yaml.load(stream, OrderedLoader)
 
 
 def _ordered_dump(data, stream=None, Dumper=yaml.Dumper, **kwds):

From b3261309843f8cfec1a5a589b4628e44e6bf76df Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 11:32:53 +0200
Subject: [PATCH 12/23] import ordering

---
 cron/parse_builds.py                                        | 2 +-
 cron/parse_builds_3_sites.py                                | 3 ++-
 lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py | 3 ++-
 lib/galaxy/tools/data/__init__.py                           | 3 ++-
 lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py           | 3 ++-
 lib/tool_shed/capsule/capsule_manager.py                    | 3 ++-
 6 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/cron/parse_builds.py b/cron/parse_builds.py
index e728315be13..2e4149fce57 100644
--- a/cron/parse_builds.py
+++ b/cron/parse_builds.py
@@ -6,10 +6,10 @@ build   description
 """
 from __future__ import print_function
 
-import requests
 import sys
 import xml.etree.ElementTree as ElementTree
 
+import requests
 
 def getbuilds(url):
     try:
diff --git a/cron/parse_builds_3_sites.py b/cron/parse_builds_3_sites.py
index 464064f4736..7d3af104420 100644
--- a/cron/parse_builds_3_sites.py
+++ b/cron/parse_builds_3_sites.py
@@ -4,9 +4,10 @@ Connects to sites and determines which builds are available at each.
 """
 from __future__ import print_function
 
-import requests
 import xml.etree.ElementTree as ElementTree
 
+import requests
+
 sites = ['http://genome.ucsc.edu/cgi-bin/',
          'http://archaea.ucsc.edu/cgi-bin/',
          'http://genome-test.cse.ucsc.edu/cgi-bin/']
diff --git a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
index 0adf41e5e02..297b56e90b9 100644
--- a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
+++ b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
@@ -3,11 +3,12 @@ Module for managing jobs in Pacific Bioscience's SMRT Portal and automatically t
 produced by SMRT Portal.
 """
 import logging
-import requests
 from string import Template
 
 from .data_transfer import DataTransfer
 
+import requests
+
 log = logging.getLogger(__name__)
 
 __all__ = ('SMRTPortalPlugin', )
diff --git a/lib/galaxy/tools/data/__init__.py b/lib/galaxy/tools/data/__init__.py
index 98013c5201e..f1fde0b7f40 100644
--- a/lib/galaxy/tools/data/__init__.py
+++ b/lib/galaxy/tools/data/__init__.py
@@ -11,7 +11,6 @@ import logging
 import os
 import os.path
 import re
-import requests
 import string
 import time
 from glob import glob
@@ -21,6 +20,8 @@ from galaxy import util
 from galaxy.util.dictifiable import Dictifiable
 from galaxy.util.odict import odict
 
+import requests
+
 log = logging.getLogger(__name__)
 
 DEFAULT_TABLE_TYPE = 'tabular'
diff --git a/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py b/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py
index 31e5c922a6d..c8562f959d4 100644
--- a/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py
+++ b/lib/galaxy/tools/imp_exp/unpack_tar_gz_archive.py
@@ -10,12 +10,13 @@ from __future__ import print_function
 import math
 import optparse
 import os
-import requests
 import sys
 import tarfile
 import tempfile
 from base64 import b64decode
 
+import requests
+
 # Set max size of archive/file that will be handled to be 100 GB. This is
 # arbitrary and should be adjusted as needed.
 MAX_SIZE = 100 * math.pow(2, 30)
diff --git a/lib/tool_shed/capsule/capsule_manager.py b/lib/tool_shed/capsule/capsule_manager.py
index dcd192ce2f4..c07d1d5de95 100644
--- a/lib/tool_shed/capsule/capsule_manager.py
+++ b/lib/tool_shed/capsule/capsule_manager.py
@@ -1,7 +1,6 @@
 import contextlib
 import logging
 import os
-import requests
 import shutil
 import tarfile
 import tempfile
@@ -21,6 +20,8 @@ from tool_shed.metadata import repository_metadata_manager
 from tool_shed.util import (basic_util, commit_util, common_util, encoding_util,
     hg_util, metadata_util, repository_util, shed_util_common as suc, xml_util)
 
+import requests
+
 log = logging.getLogger(__name__)
 
 

From 8f4491bfffce5d13e2199df5d60e06e3672a08c1 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 11:57:17 +0200
Subject: [PATCH 13/23] Consume stderr

---
 lib/galaxy/datatypes/binary.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py
index 0f8184cd1d1..924699a13a6 100644
--- a/lib/galaxy/datatypes/binary.py
+++ b/lib/galaxy/datatypes/binary.py
@@ -254,7 +254,7 @@ class Bam(Binary):
 
         # Get the version of samtools via --version-only, if available
         try:
-            output = subprocess.check_output(['samtools', '--version-only'])
+            output = subprocess.check_output(['samtools', '--version-only'], stderr=subprocess.PIPE)
             # --version-only is available
             # Format is <version x.y.z>+htslib-<a.b.c>
             version = output.split('+')[0]
@@ -263,7 +263,7 @@ class Bam(Binary):
             # --version-only not available
             pass
 
-        output = subprocess.check_output(['samtools'])
+        output = subprocess.check_output(['samtools'], stderr=subprocess.PIPE)
         lines = output.split('\n')
         for line in lines:
             if line.lower().startswith('version'):

From 0f7dd52988269dc4567bdbbd71be0fd273741efd Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Wed, 13 Sep 2017 11:57:50 +0200
Subject: [PATCH 14/23] Copy changes over

---
 lib/galaxy/datatypes/converters/sam_to_bam.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/galaxy/datatypes/converters/sam_to_bam.py b/lib/galaxy/datatypes/converters/sam_to_bam.py
index dcc84b6c518..011f660b922 100644
--- a/lib/galaxy/datatypes/converters/sam_to_bam.py
+++ b/lib/galaxy/datatypes/converters/sam_to_bam.py
@@ -34,18 +34,17 @@ def _get_samtools_version():
     if not cmd_exists('samtools'):
         raise Exception('This tool needs samtools, but it is not on PATH.')
     # Get the version of samtools via --version-only, if available
-    p = subprocess.Popen(['samtools', '--version-only'],
-                         stdout=subprocess.PIPE,
-                         stderr=subprocess.PIPE)
-    output, error = p.communicate()
-
-    # --version-only is available
-    # Format is <version x.y.z>+htslib-<a.b.c>
-    if p.returncode == 0:
+    try:
+        output = subprocess.check_output(['samtools', '--version-only'], stderr=subprocess.PIPE)
+        # --version-only is available
+        # Format is <version x.y.z>+htslib-<a.b.c>
         version = output.split('+')[0]
         return version
+    except subprocess.CalledProcessError:
+        # --version-only not available
+        pass
 
-    output = subprocess.Popen(['samtools'], stderr=subprocess.PIPE, stdout=subprocess.PIPE).communicate()[1]
+    output = subprocess.check_output(['samtools'], stderr=subprocess.PIPE)
     lines = output.split('\n')
     for line in lines:
         if line.lower().startswith('version'):

From 75df58ae86ff1796f0bb0ea975fc1d0d518c155a Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Thu, 14 Sep 2017 17:23:33 +0200
Subject: [PATCH 15/23] fix

---
 cron/parse_builds_3_sites.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cron/parse_builds_3_sites.py b/cron/parse_builds_3_sites.py
index 7d3af104420..4ae43e52765 100644
--- a/cron/parse_builds_3_sites.py
+++ b/cron/parse_builds_3_sites.py
@@ -20,11 +20,11 @@ def main():
         trackurl = sites[i] + "hgTracks?"
         builds = []
         try:
-            page = requests.get(site)
+            text = requests.get(site).text
         except:
             print("#Unable to connect to " + site)
             continue
-        text = page.text
+
         try:
             tree = ElementTree.fromstring(text)
         except:

From fd92ac3eedd145635a74530ba62bb7fea7c540f4 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Thu, 14 Sep 2017 17:23:40 +0200
Subject: [PATCH 16/23] revert this change

---
 lib/galaxy/datatypes/binary.py | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py
index 924699a13a6..1cb5eb73686 100644
--- a/lib/galaxy/datatypes/binary.py
+++ b/lib/galaxy/datatypes/binary.py
@@ -252,18 +252,15 @@ class Bam(Binary):
             message = 'Attempting to use functionality requiring samtools, but it cannot be located on Galaxy\'s PATH.'
             raise Exception(message)
 
-        # Get the version of samtools via --version-only, if available
-        try:
-            output = subprocess.check_output(['samtools', '--version-only'], stderr=subprocess.PIPE)
-            # --version-only is available
-            # Format is <version x.y.z>+htslib-<a.b.c>
+        p = subprocess.Popen(['samtools', '--version-only'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        output, error = p.communicate()
+        # --version-only is available
+        # Format is <version x.y.z>+htslib-<a.b.c>
+        if p.returncode == 0:
             version = output.split('+')[0]
             return version
-        except subprocess.CalledProcessError:
-            # --version-only not available
-            pass
 
-        output = subprocess.check_output(['samtools'], stderr=subprocess.PIPE)
+        output = subprocess.Popen(['samtools'], stderr=subprocess.PIPE, stdout=subprocess.PIPE).communicate()[1]
         lines = output.split('\n')
         for line in lines:
             if line.lower().startswith('version'):

From 27beb7030d99a511a428ad59211a3137a2761559 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Thu, 14 Sep 2017 17:25:40 +0200
Subject: [PATCH 17/23] revert/sync

---
 lib/galaxy/datatypes/binary.py                |  2 +-
 lib/galaxy/datatypes/converters/sam_to_bam.py | 14 ++++++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py
index 1cb5eb73686..87cf9aeee92 100644
--- a/lib/galaxy/datatypes/binary.py
+++ b/lib/galaxy/datatypes/binary.py
@@ -1302,7 +1302,7 @@ class ExcelXls(Binary):
     edam_format = "format_3468"
 
     def sniff(self, filename):
-        mime_type = subprocess.check_output(['file', '--mime-type', filename]).strip()
+        mime_type = subprocess.check_output(['file', '--mime-type', filename])
         return "application/vnd.ms-excel" in mime_type
 
     def get_mime(self):
diff --git a/lib/galaxy/datatypes/converters/sam_to_bam.py b/lib/galaxy/datatypes/converters/sam_to_bam.py
index 011f660b922..8c54258eb9a 100644
--- a/lib/galaxy/datatypes/converters/sam_to_bam.py
+++ b/lib/galaxy/datatypes/converters/sam_to_bam.py
@@ -34,17 +34,15 @@ def _get_samtools_version():
     if not cmd_exists('samtools'):
         raise Exception('This tool needs samtools, but it is not on PATH.')
     # Get the version of samtools via --version-only, if available
-    try:
-        output = subprocess.check_output(['samtools', '--version-only'], stderr=subprocess.PIPE)
-        # --version-only is available
-        # Format is <version x.y.z>+htslib-<a.b.c>
+    p = subprocess.Popen(['samtools', '--version-only'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    output, error = p.communicate()
+    # --version-only is available
+    # Format is <version x.y.z>+htslib-<a.b.c>
+    if p.returncode == 0:
         version = output.split('+')[0]
         return version
-    except subprocess.CalledProcessError:
-        # --version-only not available
-        pass
 
-    output = subprocess.check_output(['samtools'], stderr=subprocess.PIPE)
+    output = subprocess.Popen(['samtools'], stderr=subprocess.PIPE, stdout=subprocess.PIPE).communicate()[1]
     lines = output.split('\n')
     for line in lines:
         if line.lower().startswith('version'):

From 1a57c06d6a81dc6f92eb4843c89a7333f874bc32 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Thu, 14 Sep 2017 17:54:32 +0200
Subject: [PATCH 18/23] Fix import ordering

Thanks @nsoranzo, I appreciate all the comments here.
---
 lib/galaxy/datatypes/tabular.py                             | 2 +-
 lib/galaxy/datatypes/text.py                                | 5 +++--
 lib/galaxy/external_services/actions.py                     | 2 +-
 lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py | 4 ++--
 lib/galaxy/jobs/transfer_manager.py                         | 6 ++++--
 lib/galaxy/objectstore/s3.py                                | 6 +++---
 lib/galaxy/tools/data/__init__.py                           | 4 ++--
 lib/galaxy/web/proxy/__init__.py                            | 3 ++-
 lib/galaxy/webapps/config_manage.py                         | 2 +-
 lib/galaxy/webapps/galaxy/controllers/async.py              | 1 +
 lib/galaxy/webapps/galaxy/controllers/library_common.py     | 6 ++----
 lib/galaxy/webapps/galaxy/controllers/root.py               | 2 +-
 lib/galaxy/webapps/tool_shed/controllers/upload.py          | 3 ++-
 lib/tool_shed/capsule/capsule_manager.py                    | 3 +--
 14 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/lib/galaxy/datatypes/tabular.py b/lib/galaxy/datatypes/tabular.py
index 9a6f0eb2cb5..234a436a648 100644
--- a/lib/galaxy/datatypes/tabular.py
+++ b/lib/galaxy/datatypes/tabular.py
@@ -524,7 +524,7 @@ class Sam(Tabular):
         shutil.move(split_files[0], output_file)
 
         if len(split_files) > 1:
-            cmd = ['egrep', '-v', '-h' '^@'] + split_files[1:] + ['>>', output_file]
+            cmd = ['egrep', '-v', '-h', '^@'] + split_files[1:] + ['>>', output_file]
             subprocess.check_call(cmd, shell=True)
 
     merge = staticmethod(merge)
diff --git a/lib/galaxy/datatypes/text.py b/lib/galaxy/datatypes/text.py
index c201b872b1f..3b72e3d48b0 100644
--- a/lib/galaxy/datatypes/text.py
+++ b/lib/galaxy/datatypes/text.py
@@ -10,6 +10,8 @@ import re
 import subprocess
 import tempfile
 
+from six.moves import shlex_quote
+
 from galaxy.datatypes.data import get_file_peek, Text
 from galaxy.datatypes.metadata import MetadataElement, MetadataParameter
 from galaxy.datatypes.sniff import iter_headers
@@ -149,12 +151,11 @@ class Ipynb(Json):
             ofile_handle.close()
             try:
                 cmd = ['jupyter', 'nbconvert', '--to', 'html', '--template', 'full', dataset.file_name, '--output', ofilename]
-                log.info("Calling command %s", ' '.join(cmd))
                 subprocess.check_call(cmd)
                 ofilename = '%s.html' % ofilename
             except subprocess.CalledProcessError:
                 ofilename = dataset.file_name
-                log.exception('Command "%s" failed. Could not convert the Jupyter Notebook to HTML, defaulting to plain text.', cmd)
+                log.exception('Command "%s" failed. Could not convert the Jupyter Notebook to HTML, defaulting to plain text.', map(shlex_quote, cmd))
             return open(ofilename)
 
     def set_meta(self, dataset, **kwd):
diff --git a/lib/galaxy/external_services/actions.py b/lib/galaxy/external_services/actions.py
index 71520265c49..13dc83703d3 100644
--- a/lib/galaxy/external_services/actions.py
+++ b/lib/galaxy/external_services/actions.py
@@ -104,7 +104,7 @@ class ExternalServiceWebAPIActionResult(ExternalServiceResult):
     @property
     def content(self):
         if self._content is None:
-            self._content = requests.get(self.url).read()
+            self._content = requests.get(self.url).text
         return self._content
 
 
diff --git a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
index 297b56e90b9..b90f9940bcd 100644
--- a/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
+++ b/lib/galaxy/jobs/deferred/pacific_biosciences_smrt_portal.py
@@ -5,10 +5,10 @@ produced by SMRT Portal.
 import logging
 from string import Template
 
-from .data_transfer import DataTransfer
-
 import requests
 
+from .data_transfer import DataTransfer
+
 log = logging.getLogger(__name__)
 
 __all__ = ('SMRTPortalPlugin', )
diff --git a/lib/galaxy/jobs/transfer_manager.py b/lib/galaxy/jobs/transfer_manager.py
index 8f3cd4539ee..6bca709af3b 100644
--- a/lib/galaxy/jobs/transfer_manager.py
+++ b/lib/galaxy/jobs/transfer_manager.py
@@ -9,6 +9,8 @@ import socket
 import subprocess
 import threading
 
+from six.moves import shlex_quote
+
 from galaxy.util import listify, sleeper
 from galaxy.util.json import jsonrpc_request, validate_jsonrpc_response
 
@@ -68,8 +70,8 @@ class TransferManager(object):
             # The transfer script should daemonize fairly quickly - if this is
             # not the case, this process will need to be moved to a
             # non-blocking method.
-            cmd = self.command + [tj.id]
-            log.debug('Transfer command is: %s', ' '.join(cmd))
+            cmd = self.command.append(tj.id)
+            log.debug('Transfer command is: %s', ' '.join(map(shlex_quote, cmd)))
             p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
             p.wait()
             output = p.stdout.read(32768)
diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py
index 0d8297316e8..34a2fc39d74 100644
--- a/lib/galaxy/objectstore/s3.py
+++ b/lib/galaxy/objectstore/s3.py
@@ -18,6 +18,7 @@ from galaxy.util import (
     safe_relpath,
     string_as_bool,
     umask_fix_perms,
+    which,
 )
 from galaxy.util.sleeper import Sleeper
 
@@ -67,10 +68,9 @@ class S3ObjectStore(ObjectStore):
             self.cache_monitor_thread.start()
             log.info("Cache cleaner manager started")
         # Test if 'axel' is available for parallel download and pull the key into cache
-        try:
-            subprocess.check_call(['axel'])
+        if which('axel'):
             self.use_axel = True
-        except OSError:
+        else:
             self.use_axel = False
 
     def _configure_connection(self):
diff --git a/lib/galaxy/tools/data/__init__.py b/lib/galaxy/tools/data/__init__.py
index f1fde0b7f40..d3ec66072b7 100644
--- a/lib/galaxy/tools/data/__init__.py
+++ b/lib/galaxy/tools/data/__init__.py
@@ -16,12 +16,12 @@ import time
 from glob import glob
 from tempfile import NamedTemporaryFile
 
+import requests
+
 from galaxy import util
 from galaxy.util.dictifiable import Dictifiable
 from galaxy.util.odict import odict
 
-import requests
-
 log = logging.getLogger(__name__)
 
 DEFAULT_TABLE_TYPE = 'tabular'
diff --git a/lib/galaxy/web/proxy/__init__.py b/lib/galaxy/web/proxy/__init__.py
index fcb23ba56c0..581050a3d5e 100644
--- a/lib/galaxy/web/proxy/__init__.py
+++ b/lib/galaxy/web/proxy/__init__.py
@@ -8,9 +8,10 @@ from galaxy.util import sockets
 from galaxy.util.lazy_process import LazyProcess, NoOpLazyProcess
 from galaxy.util import sqlite
 from galaxy.util import unique_id
-import requests
 import time
 
+import requests
+
 log = logging.getLogger(__name__)
 
 
diff --git a/lib/galaxy/webapps/config_manage.py b/lib/galaxy/webapps/config_manage.py
index ee7b6b38069..685b5abcf56 100644
--- a/lib/galaxy/webapps/config_manage.py
+++ b/lib/galaxy/webapps/config_manage.py
@@ -11,8 +11,8 @@ import string
 import sys
 import tempfile
 from textwrap import TextWrapper
-import requests
 
+import requests
 import six
 from six import StringIO
 
diff --git a/lib/galaxy/webapps/galaxy/controllers/async.py b/lib/galaxy/webapps/galaxy/controllers/async.py
index 001e832aea5..aabb551bdfc 100644
--- a/lib/galaxy/webapps/galaxy/controllers/async.py
+++ b/lib/galaxy/webapps/galaxy/controllers/async.py
@@ -4,6 +4,7 @@ Upload class
 
 import logging
 import urllib
+
 import requests
 
 from galaxy import jobs, web
diff --git a/lib/galaxy/webapps/galaxy/controllers/library_common.py b/lib/galaxy/webapps/galaxy/controllers/library_common.py
index a35d6a261ef..2a1ac0a09dc 100644
--- a/lib/galaxy/webapps/galaxy/controllers/library_common.py
+++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py
@@ -4,7 +4,6 @@ import logging
 import operator
 import os
 import os.path
-import requests
 import string
 import sys
 import tarfile
@@ -12,6 +11,7 @@ import tempfile
 import urllib
 import zipfile
 
+import requests
 from markupsafe import escape
 from sqlalchemy import and_, false
 from sqlalchemy.orm import eagerload_all
@@ -2762,9 +2762,7 @@ def lucene_search(trans, cntrller, search_term, search_url, **kwd):
     message = escape(kwd.get('message', ''))
     status = kwd.get('status', 'done')
     full_url = "%s/find?%s" % (search_url, urllib.urlencode({"kwd" : search_term}))
-    response = requests.get(full_url).text
-    ldda_ids = json.loads(response)["ids"]
-    response.close()
+    ldda_ids = requests.get(full_url).json()['ids']
     lddas = [trans.sa_session.query(trans.app.model.LibraryDatasetDatasetAssociation).get(ldda_id) for ldda_id in ldda_ids]
     return status, message, get_sorted_accessible_library_items(trans, cntrller, lddas, 'name')
 
diff --git a/lib/galaxy/webapps/galaxy/controllers/root.py b/lib/galaxy/webapps/galaxy/controllers/root.py
index 6913f705080..a8d2df5f953 100644
--- a/lib/galaxy/webapps/galaxy/controllers/root.py
+++ b/lib/galaxy/webapps/galaxy/controllers/root.py
@@ -3,8 +3,8 @@ Contains the main interface in the Universe class
 """
 import cgi
 import os
-import requests
 
+import requests
 from paste.httpexceptions import HTTPNotFound, HTTPBadGateway
 
 from galaxy import web
diff --git a/lib/galaxy/webapps/tool_shed/controllers/upload.py b/lib/galaxy/webapps/tool_shed/controllers/upload.py
index b4c4dccd8d5..2fa3d3be406 100644
--- a/lib/galaxy/webapps/tool_shed/controllers/upload.py
+++ b/lib/galaxy/webapps/tool_shed/controllers/upload.py
@@ -1,10 +1,11 @@
 import logging
 import os
-import requests
 import shutil
 import tarfile
 import tempfile
 
+import requests
+
 from galaxy import util
 from galaxy import web
 from galaxy.util import checkers
diff --git a/lib/tool_shed/capsule/capsule_manager.py b/lib/tool_shed/capsule/capsule_manager.py
index c07d1d5de95..5daa0b21c04 100644
--- a/lib/tool_shed/capsule/capsule_manager.py
+++ b/lib/tool_shed/capsule/capsule_manager.py
@@ -7,6 +7,7 @@ import tempfile
 import threading
 from time import gmtime, strftime
 
+import requests
 from sqlalchemy import and_, false
 
 import tool_shed.repository_types.util as rt_util
@@ -20,8 +21,6 @@ from tool_shed.metadata import repository_metadata_manager
 from tool_shed.util import (basic_util, commit_util, common_util, encoding_util,
     hg_util, metadata_util, repository_util, shed_util_common as suc, xml_util)
 
-import requests
-
 log = logging.getLogger(__name__)
 
 

From 290971ca7a28eb579dc2d40c9653b9e160253634 Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Mon, 18 Sep 2017 10:32:49 +0200
Subject: [PATCH 19/23] more review comments

---
 cron/build_chrom_db.py                       |  2 +-
 cron/parse_builds.py                         |  1 +
 lib/galaxy/datatypes/text.py                 |  2 +-
 lib/galaxy/jobs/transfer_manager.py          |  2 +-
 scripts/data_libraries/build_lucene_index.py |  1 +
 scripts/edam_mapping.py                      |  1 +
 scripts/microbes/harvest_bacteria.py         |  2 +-
 scripts/tool_shed/api/export.py              | 21 ++++++--------------
 8 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/cron/build_chrom_db.py b/cron/build_chrom_db.py
index d3cba98b8dc..4775897548f 100644
--- a/cron/build_chrom_db.py
+++ b/cron/build_chrom_db.py
@@ -15,9 +15,9 @@ from __future__ import print_function
 
 import fileinput
 import os
-import requests
 import sys
 
+import requests
 from six.moves.urllib.parse import urlencode
 
 import parse_builds
diff --git a/cron/parse_builds.py b/cron/parse_builds.py
index 2e4149fce57..243440a331b 100644
--- a/cron/parse_builds.py
+++ b/cron/parse_builds.py
@@ -11,6 +11,7 @@ import xml.etree.ElementTree as ElementTree
 
 import requests
 
+
 def getbuilds(url):
     try:
         text = requests.get(url).text
diff --git a/lib/galaxy/datatypes/text.py b/lib/galaxy/datatypes/text.py
index 3b72e3d48b0..12235e566c4 100644
--- a/lib/galaxy/datatypes/text.py
+++ b/lib/galaxy/datatypes/text.py
@@ -155,7 +155,7 @@ class Ipynb(Json):
                 ofilename = '%s.html' % ofilename
             except subprocess.CalledProcessError:
                 ofilename = dataset.file_name
-                log.exception('Command "%s" failed. Could not convert the Jupyter Notebook to HTML, defaulting to plain text.', map(shlex_quote, cmd))
+                log.exception('Command "%s" failed. Could not convert the Jupyter Notebook to HTML, defaulting to plain text.', ' '.join(map(shlex_quote, cmd)))
             return open(ofilename)
 
     def set_meta(self, dataset, **kwd):
diff --git a/lib/galaxy/jobs/transfer_manager.py b/lib/galaxy/jobs/transfer_manager.py
index 6bca709af3b..8e06024832c 100644
--- a/lib/galaxy/jobs/transfer_manager.py
+++ b/lib/galaxy/jobs/transfer_manager.py
@@ -70,7 +70,7 @@ class TransferManager(object):
             # The transfer script should daemonize fairly quickly - if this is
             # not the case, this process will need to be moved to a
             # non-blocking method.
-            cmd = self.command.append(tj.id)
+            cmd = self.command + [tj.id]
             log.debug('Transfer command is: %s', ' '.join(map(shlex_quote, cmd)))
             p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
             p.wait()
diff --git a/scripts/data_libraries/build_lucene_index.py b/scripts/data_libraries/build_lucene_index.py
index 710747068be..b3f787e8b02 100644
--- a/scripts/data_libraries/build_lucene_index.py
+++ b/scripts/data_libraries/build_lucene_index.py
@@ -14,6 +14,7 @@ import csv
 import os
 import sys
 import urllib
+
 import requests
 
 sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, 'lib')))
diff --git a/scripts/edam_mapping.py b/scripts/edam_mapping.py
index 7e70c5467cd..bb5e495aa46 100644
--- a/scripts/edam_mapping.py
+++ b/scripts/edam_mapping.py
@@ -17,6 +17,7 @@ from __future__ import print_function
 import os
 import sys
 import requests
+
 from xml import etree
 
 sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib')))
diff --git a/scripts/microbes/harvest_bacteria.py b/scripts/microbes/harvest_bacteria.py
index 175ce70cb0d..3226dec78ce 100644
--- a/scripts/microbes/harvest_bacteria.py
+++ b/scripts/microbes/harvest_bacteria.py
@@ -8,9 +8,9 @@ import os
 import sys
 import time
 from ftplib import FTP
-import requests
 from urllib import urlretrieve
 
+import requests
 from BeautifulSoup import BeautifulSoup
 from util import get_bed_from_genbank, get_bed_from_glimmer3, get_bed_from_GeneMarkHMM, get_bed_from_GeneMark
 
diff --git a/scripts/tool_shed/api/export.py b/scripts/tool_shed/api/export.py
index 68005de0746..6a5d030c6a0 100644
--- a/scripts/tool_shed/api/export.py
+++ b/scripts/tool_shed/api/export.py
@@ -11,6 +11,7 @@ import argparse
 import os
 import sys
 import tempfile
+
 import requests
 
 sys.path.insert(1, os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, os.pardir, 'lib'))
@@ -98,21 +99,11 @@ def main(options):
             download_url = export_dict['download_url']
             download_dir = os.path.abspath(options.download_dir)
             file_path = os.path.join(download_dir, repositories_archive_filename)
-            src = None
-            dst = None
-            try:
-                src = requests.get(download_url, stream=True)
-                with open(file_path, 'wb') as handle:
-                    for chunk in src.iter_content(chunk_size=CHUNK_SIZE):
-                        if chunk:
-                            handle.write(chunk)
-            except:
-                raise
-            finally:
-                if src:
-                    src.close()
-                if dst:
-                    dst.close()
+            src = requests.get(download_url, stream=True)
+            with open(file_path, 'wb') as dst:
+                for chunk in src.iter_content(chunk_size=CHUNK_SIZE):
+                    if chunk:
+                        dst.write(chunk)
             print "Successfully exported revision ", options.changeset_revision, " of repository ", options.name, " owned by ", options.owner
             print "to location ", file_path
     else:

From 15c73233c52b29ed94066c06ebb1e10c935ae28d Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Mon, 18 Sep 2017 11:32:02 +0200
Subject: [PATCH 20/23] fix lint

---
 scripts/edam_mapping.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/edam_mapping.py b/scripts/edam_mapping.py
index bb5e495aa46..f483db8c1f5 100644
--- a/scripts/edam_mapping.py
+++ b/scripts/edam_mapping.py
@@ -16,10 +16,10 @@ from __future__ import print_function
 
 import os
 import sys
-import requests
-
 from xml import etree
 
+import requests
+
 sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib')))
 
 import galaxy.model

From 19d03edc7ff8384a5360d71365076a2594fb0b8b Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Mon, 18 Sep 2017 11:33:53 +0200
Subject: [PATCH 21/23] refactor missed call

---
 lib/galaxy/tools/deps/mulled/mulled_build_channel.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/tools/deps/mulled/mulled_build_channel.py b/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
index ff87189b65b..8338fb51053 100644
--- a/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
+++ b/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
@@ -21,13 +21,13 @@ from __future__ import print_function
 import os
 import sys
 import time
+import subprocess
 
 from ._cli import arg_parser
 from .mulled_build import (
     add_build_arguments,
     args_to_mull_targets_kwds,
     build_target,
-    check_output,
     conda_versions,
     get_affected_packages,
     mull_targets,
@@ -42,7 +42,13 @@ def _fetch_repo_data(args):
         repo_data = "%s-repodata.json" % channel
     if not os.path.exists(repo_data):
         platform_tag = 'osx-64' if sys.platform == 'darwin' else 'linux-64'
-        check_output("wget --quiet https://conda.anaconda.org/%s/%s/repodata.json.bz2 -O '%s.bz2' && bzip2 -d '%s.bz2'" % (channel, platform_tag, repo_data, repo_data))
+        subprocess.check_call([
+            'wget', '--quiet', 'https://conda.anaconda.org/%s/%s/repodata.json.bz2' % (channel, platform_tag),
+            '-O', '%s.bz2' % repo_data
+        ])
+        subprocess.check_call([
+            'bzip2', '-d', '%s.bz2' % repo_data
+        ])
     return repo_data
 
 

From 626a3dc2995f0d797232aa666f1bdf3e6a5d22dd Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Mon, 18 Sep 2017 13:16:06 +0200
Subject: [PATCH 22/23] fix ordering

---
 lib/galaxy/tools/deps/mulled/mulled_build_channel.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tools/deps/mulled/mulled_build_channel.py b/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
index 8338fb51053..06e0c7afe6f 100644
--- a/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
+++ b/lib/galaxy/tools/deps/mulled/mulled_build_channel.py
@@ -19,9 +19,9 @@ See recent changes that would be built with:
 from __future__ import print_function
 
 import os
+import subprocess
 import sys
 import time
-import subprocess
 
 from ._cli import arg_parser
 from .mulled_build import (

From 41574d022249bae4df4b89bfb50c3a599ee5585e Mon Sep 17 00:00:00 2001
From: E Rasche <hxr@hx42.org>
Date: Mon, 25 Sep 2017 17:32:05 +0200
Subject: [PATCH 23/23] swap for subprocess.call per @nsoranzo suggestion

---
 lib/galaxy/jobs/runners/util/job_script/__init__.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/jobs/runners/util/job_script/__init__.py b/lib/galaxy/jobs/runners/util/job_script/__init__.py
index 4c4c6b86e45..2c717c8f13d 100644
--- a/lib/galaxy/jobs/runners/util/job_script/__init__.py
+++ b/lib/galaxy/jobs/runners/util/job_script/__init__.py
@@ -119,9 +119,8 @@ def _handle_script_integrity(path, config):
     sleep_amt = getattr(config, "check_job_script_integrity_sleep", DEFAULT_INTEGRITY_SLEEP)
     for i in range(count):
         try:
-            proc = subprocess.Popen([path], env={"ABC_TEST_JOB_SCRIPT_INTEGRITY_XYZ": "1"})
-            proc.wait()
-            if proc.returncode == 42:
+            returncode = subprocess.call([path], env={"ABC_TEST_JOB_SCRIPT_INTEGRITY_XYZ": "1"})
+            if returncode == 42:
                 script_integrity_verified = True
                 break