Merge pull request #1412 from jmchilton/isolate

Isolate tool commands by default.

config/galaxy.ini.sample

@@ -860,9 +860,10 @@ use_interactive = True
 #enable_sequencer_communication = False
 
 # Separate tool command from rest of job script so tool dependencies
-# don't interfer with metadata generation (this will be the default
-# in a future release).
-#enable_beta_tool_command_isolation = False
+# don't interfer with metadata generation.
+# Despite the name, this feature is enabled by default
+# in 16.01 and the option will go away in the future.
+#enable_beta_tool_command_isolation = True
 
 
 # Set the following to a number of threads greater than 1 to spawn

lib/galaxy/config.py

@@ -215,7 +215,7 @@ class Configuration( object ):
         # Tasked job runner.
         self.use_tasked_jobs = string_as_bool( kwargs.get( 'use_tasked_jobs', False ) )
         self.local_task_queue_workers = int(kwargs.get("local_task_queue_workers", 2))
-        self.commands_in_new_shell = string_as_bool( kwargs.get( 'enable_beta_tool_command_isolation', "False" ) )
+        self.commands_in_new_shell = string_as_bool( kwargs.get( 'enable_beta_tool_command_isolation', "True" ) )
         self.tool_submission_burst_threads = int( kwargs.get( 'tool_submission_burst_threads', '1' ) )
         self.tool_submission_burst_at = int( kwargs.get( 'tool_submission_burst_at', '10' ) )
         # The transfer manager and deferred job queue

lib/galaxy/datatypes/binary.py

@@ -16,7 +16,7 @@ from bx.seq.twobit import TWOBIT_MAGIC_NUMBER, TWOBIT_MAGIC_NUMBER_SWAP, TWOBIT_
 
 from galaxy.datatypes.metadata import MetadataElement, MetadataParameter, ListParameter, DictParameter
 from galaxy.datatypes import metadata
-from galaxy.util import nice_size, sqlite
+from galaxy.util import nice_size, sqlite, which
 from . import data, dataproviders
 
 
@@ -173,6 +173,11 @@ class Bam( Binary ):
         # Determine the version of samtools being used.  Wouldn't it be nice if
         # samtools provided a version flag to make this much simpler?
         version = '0.0.0'
+        samtools_exec = which('samtools')
+        if not samtools_exec:
+            message = 'Attempting to use functionality requiring samtools, but it cannot be located on Galaxy\'s PATH.'
+            raise Exception(message)
+
         output = subprocess.Popen( [ 'samtools' ], stderr=subprocess.PIPE, stdout=subprocess.PIPE ).communicate()[1]
         lines = output.split( '\n' )
         for line in lines:

lib/galaxy/jobs/__init__.py

@@ -1569,6 +1569,7 @@ class JobWrapper( object ):
     def setup_external_metadata( self, exec_dir=None, tmp_dir=None,
                                  dataset_files_path=None, config_root=None,
                                  config_file=None, datatypes_config=None,
+                                 resolve_metadata_dependencies=False,
                                  set_extension=True, **kwds ):
         # extension could still be 'auto' if this is the upload tool.
         job = self.get_job()
@@ -1589,19 +1590,27 @@ class JobWrapper( object ):
             config_file = self.app.config.config_file
         if datatypes_config is None:
             datatypes_config = self.app.datatypes_registry.integrated_datatypes_configs
-        return self.external_output_metadata.setup_external_metadata( [ output_dataset_assoc.dataset for
-                                                                        output_dataset_assoc in
-                                                                        job.output_datasets + job.output_library_datasets ],
-                                                                      self.sa_session,
-                                                                      exec_dir=exec_dir,
-                                                                      tmp_dir=tmp_dir,
-                                                                      dataset_files_path=dataset_files_path,
-                                                                      config_root=config_root,
-                                                                      config_file=config_file,
-                                                                      datatypes_config=datatypes_config,
-                                                                      job_metadata=os.path.join( self.working_directory, TOOL_PROVIDED_JOB_METADATA_FILE ),
-                                                                      max_metadata_value_size=self.app.config.max_metadata_value_size,
-                                                                      **kwds )
+        base_command = self.external_output_metadata.setup_external_metadata( [ output_dataset_assoc.dataset for
+                                                                                output_dataset_assoc in
+                                                                                job.output_datasets + job.output_library_datasets ],
+                                                                              self.sa_session,
+                                                                              exec_dir=exec_dir,
+                                                                              tmp_dir=tmp_dir,
+                                                                              dataset_files_path=dataset_files_path,
+                                                                              config_root=config_root,
+                                                                              config_file=config_file,
+                                                                              datatypes_config=datatypes_config,
+                                                                              job_metadata=os.path.join( self.working_directory, TOOL_PROVIDED_JOB_METADATA_FILE ),
+                                                                              max_metadata_value_size=self.app.config.max_metadata_value_size,
+                                                                              **kwds )
+        command = base_command
+        if resolve_metadata_dependencies:
+            metadata_tool = self.app.toolbox.get_tool("__SET_METADATA__")
+            dependency_shell_commands = metadata_tool.build_dependency_shell_commands(job_directory=self.working_directory)
+            if dependency_shell_commands:
+                dependency_shell_commands = "; ".join(dependency_shell_commands)
+                command = "%s; %s" % (dependency_shell_commands, command)
+        return command
 
     @property
     def user( self ):

lib/galaxy/jobs/command_factory.py

@@ -33,6 +33,9 @@ def build_command(
     base_command_line = job_wrapper.get_command_line()
     # job_id = job_wrapper.job_id
     # log.debug( 'Tool evaluation for job (%s) produced command-line: %s' % ( job_id, base_command_line ) )
+    if not base_command_line:
+        raise Exception("Attempting to run a tool with empty command definition.")
+
     commands_builder = CommandsBuilder(base_command_line)
 
     # All job runners currently handle this case which should never occur
@@ -75,15 +78,16 @@ def build_command(
 
 def __externalize_commands(job_wrapper, commands_builder, remote_command_params, script_name="tool_script.sh"):
     local_container_script = join( job_wrapper.working_directory, script_name )
+    tool_commands = commands_builder.build()
     with open( local_container_script, "w" ) as f:
-        script_contents = "#!%s\n%s" % (DEFAULT_SHELL, commands_builder.build())
-        f.write( script_contents )
+        script_contents = u"#!%s\n%s" % (DEFAULT_SHELL, tool_commands)
+        f.write(script_contents.encode(util.DEFAULT_ENCODING))
     chmod( local_container_script, 0755 )
 
     commands = local_container_script
     if 'working_directory' in remote_command_params:
         commands = "%s %s" % (DEFAULT_SHELL, join(remote_command_params['working_directory'], script_name))
-    log.info("Built script [%s] for tool command[%s]" % (local_container_script, commands))
+    log.info("Built script [%s] for tool command[%s]" % (local_container_script, tool_commands))
     return commands
 
 
@@ -132,6 +136,7 @@ def __handle_metadata(commands_builder, job_wrapper, runner, remote_command_para
     config_file = metadata_kwds.get( 'config_file', None )
     datatypes_config = metadata_kwds.get( 'datatypes_config', None )
     compute_tmp_dir = metadata_kwds.get( 'compute_tmp_dir', None )
+    resolve_metadata_dependencies = job_wrapper.commands_in_new_shell
     metadata_command = job_wrapper.setup_external_metadata(
         exec_dir=exec_dir,
         tmp_dir=tmp_dir,
@@ -142,6 +147,7 @@ def __handle_metadata(commands_builder, job_wrapper, runner, remote_command_para
         config_file=config_file,
         datatypes_config=datatypes_config,
         compute_tmp_dir=compute_tmp_dir,
+        resolve_metadata_dependencies=resolve_metadata_dependencies,
         kwds={ 'overwrite': False }
     ) or ''
     metadata_command = metadata_command.strip()
@@ -157,9 +163,7 @@ def __copy_if_exists_command(work_dir_output):
 
 class CommandsBuilder(object):
 
-    def __init__(self, initial_command):
-        if not initial_command:
-            raise Exception("Attempting to run a tool with empty command definition.")
+    def __init__(self, initial_command=u''):
         # Remove trailing semi-colon so we can start hacking up this command.
         # TODO: Refactor to compose a list and join with ';', would be more clean.
         initial_command = util.unicodify(initial_command)

lib/galaxy/tools/deps/commands.py

@@ -1,6 +1,7 @@
 import os
 import subprocess
 import sys as _sys
+from galaxy.util import which
 
 
 def redirecting_io(sys=_sys):
@@ -57,15 +58,6 @@ def execute(cmds):
     return __wait(cmds, shell=False)
 
 
-def which(file):
-    # http://stackoverflow.com/questions/5226958/which-equivalent-function-in-python
-    for path in os.environ["PATH"].split(":"):
-        if os.path.exists(path + "/" + file):
-                return path + "/" + file
-
-    return None
-
-
 def __wait(cmds, **popen_kwds):
     p = subprocess.Popen(cmds, **popen_kwds)
     stdout, stderr = p.communicate()
@@ -101,3 +93,15 @@ class CommandLineException(Exception):
 
     def __str__(self):
         return self.message
+
+
+__all__ = [
+    'CommandLineException',
+    'download_command',
+    'execute',
+    'redirect_aware_commmunicate',
+    'redirecting_io',
+    'shell',
+    'shell_process',
+    'which',
+]

lib/galaxy/util/__init__.py

@@ -555,6 +555,15 @@ def ready_name_for_url( raw_name ):
     return slug_base
 
 
+def which(file):
+    # http://stackoverflow.com/questions/5226958/which-equivalent-function-in-python
+    for path in os.environ["PATH"].split(":"):
+        if os.path.exists(path + "/" + file):
+                return path + "/" + file
+
+    return None
+
+
 def in_directory( file, directory, local_path_module=os.path ):
     """
     Return true, if the common prefix of both is equal to directory

test/unit/jobs/test_command_factory.py

@@ -1,4 +1,7 @@
+import os
 from os import getcwd
+import shutil
+from tempfile import mkdtemp
 from unittest import TestCase
 
 from galaxy.jobs.command_factory import build_command
@@ -12,7 +15,8 @@ TEST_FILES_PATH = "file_path"
 class TestCommandFactory(TestCase):
 
     def setUp(self):
-        self.job_wrapper = MockJobWrapper()
+        self.job_dir = mkdtemp()
+        self.job_wrapper = MockJobWrapper(self.job_dir)
         self.workdir_outputs = []
 
         def workdir_outputs(job_wrapper, **kwds):
@@ -23,6 +27,9 @@ class TestCommandFactory(TestCase):
         self.include_metadata = False
         self.include_work_dir_outputs = True
 
+    def tearDown(self):
+        shutil.rmtree(self.job_dir)
+
     def test_simplest_command(self):
         self.include_work_dir_outputs = False
         self.__assert_command_is( MOCK_COMMAND_LINE )
@@ -33,6 +40,14 @@ class TestCommandFactory(TestCase):
         self.job_wrapper.dependency_shell_commands = dep_commands
         self.__assert_command_is( "%s; %s" % (dep_commands[0], MOCK_COMMAND_LINE) )
 
+    def test_shell_commands_external(self):
+        self.job_wrapper.commands_in_new_shell = True
+        self.include_work_dir_outputs = False
+        dep_commands = [". /opt/galaxy/tools/bowtie/default/env.sh"]
+        self.job_wrapper.dependency_shell_commands = dep_commands
+        self.__assert_command_is( "%s/tool_script.sh" % self.job_wrapper.working_directory)
+        self.__assert_tool_script_is( "#!/bin/sh\n%s; %s" % (dep_commands[0], MOCK_COMMAND_LINE) )
+
     def test_remote_dependency_resolution(self):
         self.include_work_dir_outputs = False
         dep_commands = [". /opt/galaxy/tools/bowtie/default/env.sh"]
@@ -119,6 +134,13 @@ class TestCommandFactory(TestCase):
         command = self.__command(**command_kwds)
         self.assertEqual(command, expected_command)
 
+    def __assert_tool_script_is(self, expected_command):
+        self.assertEqual(open(self.__tool_script, "r").read(), expected_command)
+
+    @property
+    def __tool_script(self):
+        return os.path.join(self.job_dir, "tool_script.sh")
+
     def __command(self, **extra_kwds):
         kwds = dict(
             runner=self.runner,
@@ -132,13 +154,13 @@ class TestCommandFactory(TestCase):
 
 class MockJobWrapper(object):
 
-    def __init__(self):
+    def __init__(self, job_dir):
         self.write_version_cmd = None
         self.command_line = MOCK_COMMAND_LINE
         self.dependency_shell_commands = []
         self.metadata_line = None
         self.configured_external_metadata_kwds = None
-        self.working_directory = "job1"
+        self.working_directory = job_dir
         self.prepare_input_files_cmds = None
         self.commands_in_new_shell = False