Skip to content

Commit

Permalink
Revert "job script returns tool exit code in any case"
Browse files Browse the repository at this point in the history 
This breaks exit code reporting and leads to pod re-scheduling.

This reverts commit 6beeec8.
  • Loading branch information
@mvdbeek
mvdbeek committed Feb 19, 2019
1 parent e52bb5d commit 626657e
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
3 changes: 3 additions & 0 deletions lib/galaxy/jobs/command_factory.py
View file
Expand Up @@ -15,6 +15,7 @@
log = getLogger(__name__)

CAPTURE_RETURN_CODE = "return_code=$?"
YIELD_CAPTURED_CODE = 'sh -c "exit $return_code"'
SETUP_GALAXY_FOR_METADATA = """
[ "$GALAXY_VIRTUAL_ENV" = "None" ] && GALAXY_VIRTUAL_ENV="$_GALAXY_VIRTUAL_ENV"; _galaxy_setup_environment True
"""
Expand Down Expand Up @@ -268,6 +269,8 @@ def capture_return_code(self):
self.append_command(CAPTURE_RETURN_CODE)

def build(self):
if self.return_code_captured:
self.append_command(YIELD_CAPTURED_CODE)
return self.commands


Expand Down
4 changes: 1 addition & 3 deletions lib/galaxy/jobs/runners/util/job_script/DEFAULT_JOB_FILE_TEMPLATE.sh
View file
Expand Up @@ -37,7 +37,5 @@ cd $working_directory
$memory_statement
$instrument_pre_commands
$command
echo $return_code > $exit_code_path
echo $? > $exit_code_path
$instrument_post_commands
# exit with the exit code captured for the tool_script
sh -c "exit $return_code"
2 changes: 1 addition & 1 deletion lib/galaxy/jobs/runners/util/job_script/__init__.py
View file
Expand Up @@ -67,7 +67,7 @@ def job_script(template=DEFAULT_JOB_FILE_TEMPLATE, **kwds):
>>> script = job_script(working_directory='wd', command='uptime', exit_code_path='ec')
>>> '\\nuptime\\n' in script
True
>>> 'echo $return_code > ec' in script
>>> 'echo $? > ec' in script
True
>>> 'GALAXY_LIB="None"' in script
True
Expand Down
24 changes: 12 additions & 12 deletions test/unit/jobs/test_command_factory.py
View file
Expand Up @@ -32,49 +32,49 @@ def tearDown(self):

def test_simplest_command(self):
self.include_work_dir_outputs = False
self.__assert_command_is(_surround_command(MOCK_COMMAND_LINE + "; return_code=$?"))
self.__assert_command_is(_surrond_command(MOCK_COMMAND_LINE + "; return_code=$?"))

def test_shell_commands(self):
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(_surround_command("%s; %s; return_code=$?" % (dep_commands[0], MOCK_COMMAND_LINE)))
self.__assert_command_is(_surrond_command("%s; %s; return_code=$?" % (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(_surround_command("%s/tool_script.sh; return_code=$?" % self.job_wrapper.working_directory))
self.__assert_command_is(_surrond_command("%s/tool_script.sh; return_code=$?" % 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"]
self.job_wrapper.dependency_shell_commands = dep_commands
self.__assert_command_is(_surround_command(MOCK_COMMAND_LINE + "; return_code=$?"), remote_command_params=dict(dependency_resolution="remote"))
self.__assert_command_is(_surrond_command(MOCK_COMMAND_LINE + "; return_code=$?"), remote_command_params=dict(dependency_resolution="remote"))

def test_explicit_local_dependency_resolution(self):
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(_surround_command("%s; %s; return_code=$?" % (dep_commands[0], MOCK_COMMAND_LINE)),
self.__assert_command_is(_surrond_command("%s; %s; return_code=$?" % (dep_commands[0], MOCK_COMMAND_LINE)),
remote_command_params=dict(dependency_resolution="local"))

def test_task_prepare_inputs(self):
self.include_work_dir_outputs = False
self.job_wrapper.prepare_input_files_cmds = ["/opt/split1", "/opt/split2"]
self.__assert_command_is(_surround_command("/opt/split1; /opt/split2; %s; return_code=$?") % MOCK_COMMAND_LINE)
self.__assert_command_is(_surrond_command("/opt/split1; /opt/split2; %s; return_code=$?") % MOCK_COMMAND_LINE)

def test_workdir_outputs(self):
self.include_work_dir_outputs = True
self.workdir_outputs = [("foo", "bar")]
self.__assert_command_is(_surround_command('%s; return_code=$?; if [ -f foo ] ; then cp foo bar ; fi' % MOCK_COMMAND_LINE))
self.__assert_command_is(_surrond_command('%s; return_code=$?; if [ -f foo ] ; then cp foo bar ; fi' % MOCK_COMMAND_LINE))

def test_set_metadata_skipped_if_unneeded(self):
self.include_metadata = True
self.include_work_dir_outputs = False
self.__assert_command_is(_surround_command(MOCK_COMMAND_LINE + "; return_code=$?"))
self.__assert_command_is(_surrond_command(MOCK_COMMAND_LINE + "; return_code=$?"))

def test_set_metadata(self):
self._test_set_metadata()
Expand All @@ -87,7 +87,7 @@ def _test_set_metadata(self):
self.include_metadata = True
self.include_work_dir_outputs = False
self.job_wrapper.metadata_line = TEST_METADATA_LINE
expected_command = _surround_command("%s; return_code=$?; cd '%s'; %s%s" % (MOCK_COMMAND_LINE, self.job_dir, SETUP_GALAXY_FOR_METADATA, TEST_METADATA_LINE))
expected_command = _surrond_command("%s; return_code=$?; cd '%s'; %s%s" % (MOCK_COMMAND_LINE, self.job_dir, SETUP_GALAXY_FOR_METADATA, TEST_METADATA_LINE))
self.__assert_command_is(expected_command)

def test_empty_metadata(self):
Expand All @@ -98,7 +98,7 @@ def test_empty_metadata(self):
self.include_work_dir_outputs = False
self.job_wrapper.metadata_line = ' '
# Empty metadata command do not touch command line.
expected_command = _surround_command("%s; return_code=$?; cd '%s'" % (MOCK_COMMAND_LINE, self.job_dir))
expected_command = _surrond_command("%s; return_code=$?; cd '%s'" % (MOCK_COMMAND_LINE, self.job_dir))
self.__assert_command_is(expected_command)

def test_metadata_kwd_defaults(self):
Expand Down Expand Up @@ -152,8 +152,8 @@ def __command(self, **extra_kwds):
return build_command(**kwds)


def _surround_command(command):
return '''rm -rf working; mkdir -p working; cd working; %s''' % command
def _surrond_command(command):
return '''rm -rf working; mkdir -p working; cd working; %s; sh -c "exit $return_code"''' % command


class MockJobWrapper(object):
Expand Down

0 comments on commit 626657e

Please sign in to comment.