Skip to content

Commit

Permalink
Implement profile="XX.XX" on tools, better defaults for newer tools.
Browse files Browse the repository at this point in the history
 - set -e in tool command scripts.
 - Disable implicit extra file collection. All dynamic extra file collection requires a <discover_datasets tag.
 - Disable format="input" - require explicit metadata targets (metadata_source, format_source).
 - Disable interpreter= - hacky and doesn't work as expected in too many circumstances. Use $__tool_directory__.
 - Disable $param_file - grepping the command for the string is impercise. If useful we can add flag to match the flag used to produce inputs as a JSON file.
 - Disable default version of 1.0.0, the IUC has made it clear they prefer all tools have an explicit version.

Fixes #1609.
  • Loading branch information
jmchilton committed Feb 22, 2016
1 parent d020522 commit b92074e
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 11 deletions.
15 changes: 13 additions & 2 deletions lib/galaxy/tools/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from paste import httpexceptions
from six import string_types

from galaxy.version import VERSION_MAJOR
from galaxy import model
from galaxy.managers import histories
from galaxy.datatypes.metadata import JobExternalOutputMetadataWrapper
Expand Down Expand Up @@ -460,6 +461,8 @@ def parse( self, tool_source, guid=None ):
"""
Read tool configuration from the element `root` and fill in `self`.
"""
self.legacy_defaults = tool_source.legacy_defaults
tool_profile = tool_source.parse_profile()
# Get the UNIQUE id for the tool
self.old_id = tool_source.parse_id()
if guid is None:
Expand All @@ -469,15 +472,23 @@ def parse( self, tool_source, guid=None ):
if not self.id:
raise Exception( "Missing tool 'id'" )

if not self.legacy_defaults and VERSION_MAJOR < tool_profile:
template = "The tool %s targets version %s of Galaxy, you should upgrade Galaxy to ensure proper functioning of this tool."
message = template % (self.id, tool_profile)
log.warn(message)

# Get the (user visible) name of the tool
self.name = tool_source.parse_name()
if not self.name:
raise Exception( "Missing tool 'name'" )

self.version = tool_source.parse_version()
if not self.version:
# For backward compatibility, some tools may not have versions yet.
self.version = "1.0.0"
if self.legacy_defaults:
# For backward compatibility, some tools may not have versions yet.
self.version = "1.0.0"
else:
raise Exception( "Missing tool version.")

# Support multi-byte tools
self.is_multi_byte = tool_source.parse_is_multi_byte()
Expand Down
8 changes: 6 additions & 2 deletions lib/galaxy/tools/actions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,10 @@ def execute(self, tool, trans, incoming={}, return_job=False, set_output_hid=Tru

# Deal with input dataset names, 'dbkey' and types
input_names = []
input_ext = 'data'
# format='input" previously would give you a random extension from
# the input extensions, now it should just give "input" as the output
# format.
input_ext = 'data' if tool.legacy_defaults else "input"
input_dbkey = incoming.get( "dbkey", "?" )
for name, data in reversed(inp_data.items()):
if not data:
Expand All @@ -239,7 +242,8 @@ def execute(self, tool, trans, incoming={}, return_job=False, set_output_hid=Tru
else: # HDA
if data.hid:
input_names.append( 'data %s' % data.hid )
input_ext = data.ext
if tool.legacy_defaults:
input_ext = data.ext

if data.dbkey not in [None, '?']:
input_dbkey = data.dbkey
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/tools/evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def __build_param_file( self ):
param_dict = self.param_dict
directory = self.local_working_directory
command = self.tool.command
if command and "$param_file" in command:
if self.tool.legacy_defaults and command and "$param_file" in command:
fd, param_filename = tempfile.mkstemp( dir=directory )
os.close( fd )
f = open( param_filename, "w" )
Expand Down
3 changes: 3 additions & 0 deletions lib/galaxy/tools/parser/cwl.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ def parse_requirements_and_containers(self):
containers=containers,
))

def parse_profile(self):
return "16.04"


class CwlPageSource(PageSource):

Expand Down
11 changes: 11 additions & 0 deletions lib/galaxy/tools/parser/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ class ToolSource(object):
__metaclass__ = ABCMeta
default_is_multi_byte = False

@property
def legacy_defaults(self):
# Restore oldest defaults for XML-based tools.
return self.parse_profile() == "legacy"

@abstractmethod
def parse_id(self):
""" Parse an ID describing the abstract tool. This is not the
Expand Down Expand Up @@ -158,6 +163,12 @@ def parse_help(self):
doesn't define help text.
"""

@abstractmethod
def parse_profile(self):
""" Return tool profile to use, currently this should be a Galaxy
version (> 16.04) or 'legacy'.
"""

def parse_tests_to_dict(self):
return {'tests': []}

Expand Down
4 changes: 2 additions & 2 deletions lib/galaxy/tools/parser/output_collection_def.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
LEGACY_DEFAULT_DBKEY = None # don't use __input__ for legacy default collection


def dataset_collector_descriptions_from_elem( elem ):
def dataset_collector_descriptions_from_elem( elem, legacy=True ):
primary_dataset_elems = elem.findall( "discover_datasets" )
if not primary_dataset_elems:
if len(primary_dataset_elems) == 0 and legacy:
return [ DEFAULT_DATASET_COLLECTOR_DESCRIPTION ]
else:
return map( lambda elem: DatasetCollectionDescription( **elem.attrib ), primary_dataset_elems )
Expand Down
31 changes: 27 additions & 4 deletions lib/galaxy/tools/parser/xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ def parse_environment_variables(self):

def parse_interpreter(self):
command_el = self._command_el
return (command_el is not None) and command_el.get("interpreter", None)
interpreter = (command_el is not None) and command_el.get("interpreter", None)
if not self.legacy_defaults:
log.warn("Deprecated interpeter attribute on command element is now ignored.")
interpreter = None

return interpreter

def parse_version_command(self):
version_cmd = self.root.find("version_command")
Expand Down Expand Up @@ -195,7 +200,7 @@ def _parse(data_elem, **kwds):

dataset_collector_descriptions = None
if collection_elem.find( "discover_datasets" ) is not None:
dataset_collector_descriptions = dataset_collector_descriptions_from_elem( collection_elem )
dataset_collector_descriptions = dataset_collector_descriptions_from_elem( collection_elem, legacy=False )
structure = ToolOutputCollectionStructure(
collection_type=collection_type,
collection_type_source=collection_type_source,
Expand Down Expand Up @@ -262,7 +267,7 @@ def _parse_output(
output.from_work_dir = data_elem.get("from_work_dir", None)
output.hidden = string_as_bool( data_elem.get("hidden", "") )
output.actions = ToolOutputActionGroup( output, data_elem.find( 'actions' ) )
output.dataset_collector_descriptions = dataset_collector_descriptions_from_elem( data_elem )
output.dataset_collector_descriptions = dataset_collector_descriptions_from_elem( data_elem, legacy=self.legacy_defaults )
return output

def parse_stdio(self):
Expand All @@ -277,6 +282,8 @@ def parse_stdio(self):
return aggressive_error_checks()
else:
raise ValueError("Unknown detect_errors value encountered [%s]" % detect_errors)
elif len(self.root.findall('stdio')) == 0 and not self.legacy_defaults:
return error_on_exit_code()
else:
parser = StdioParser(self.root)
return parser.stdio_exit_codes, parser.stdio_regexes
Expand All @@ -285,8 +292,10 @@ def parse_strict_shell(self):
command_el = self._command_el
if command_el is not None:
return string_as_bool(command_el.get("strict", "False"))
else:
elif self.legacy_defaults:
return False
else:
return True

def parse_help(self):
help_elem = self.root.find( 'help' )
Expand All @@ -307,6 +316,20 @@ def parse_tests_to_dict(self):

return rval

@property
def legacy_defaults(self):
# Pre-16.04 or default XML defaults
# - Use standard error for error detection.
# - Don't run shells with -e
# - Auto-check for implicit multiple outputs.
# - Auto-check for $param_file.
# - Enable buggy interpreter attribute.
return self.parse_profile() == "legacy"

def parse_profile(self):
profile = self.root.get("profile", "legacy")
return profile


def _test_elem_to_dict(test_elem, i):
rval = dict(
Expand Down
5 changes: 5 additions & 0 deletions lib/galaxy/tools/parser/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ def parse_tests_to_dict(self):

return rval

def parse_profile(self):
"""
"""
return self.root_dict.get("profile", "16.04")


def _parse_test(i, test_dict):
inputs = test_dict["inputs"]
Expand Down
1 change: 1 addition & 0 deletions test/unit/tools/test_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ def tool_directory( self ):
class MockTool( object ):

def __init__( self, app ):
self.legacy_defaults = True
self.app = app
self.hooks_called = []
self.environment_variables = []
Expand Down

0 comments on commit b92074e

Please sign in to comment.