From 7e45ca2a72aad8b6c5e94f3fbb9d470663abf15b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 31 Dec 2014 18:21:10 -0500 Subject: [PATCH] Allow multiple tools with the same id in ToolBox. How to use: 1.) Place multiple tools with different IDs in your tool conf. 2.) ... ummm ... no step 2 - just use the tools. Implementation: The Tool Shed allows tool lineages by assigning each tool version a GUID and tracking versions in a database. This implementation works by simply allowing the ToolBox to contain multiple tools with the same ID and orders them by the version specified by the tool author. To track enable this a second tool lineage has been introduced that just uses tool versions instead of a database (non-toolshed installed tools are not longer placed into the Tool Shed install database). The ToolBox has been updated to allow multiple versions per tool id (defaulting to the 'latest' version for all operations which do not specify a version). Both jobs and workflow steps would track tool versions but did not use that version when fetching tools from the Toolbox - these components have been updated to try to use the tool version. Unit tests working through most of the ToolBox and tool panel have been added, as well as functional tests exercising the tools API and to ensure workflows now at least attempt to respect tool versions (still kind of silently switches versions in some cases). Manual tests against the new tool form seem to demonstrate the tool switching and tool re-running work with only minor changes to the tools API and the job handler. --- lib/galaxy/jobs/__init__.py | 2 +- lib/galaxy/jobs/handler.py | 2 +- lib/galaxy/tools/__init__.py | 71 +++++++++++++++---- lib/galaxy/tools/toolbox/lineages/factory.py | 6 +- .../tools/toolbox/lineages/interface.py | 34 ++++++++- lib/galaxy/tools/toolbox/lineages/stock.py | 44 ++++++++++++ .../tools/toolbox/lineages/tool_shed.py | 4 ++ lib/galaxy/workflow/modules.py | 20 ++++-- test/api/test_tools.py | 20 +++++- test/api/test_workflows.py | 24 ++++++- .../tools/multiple_versions_v01.xml | 11 +++ .../tools/multiple_versions_v02.xml | 11 +++ test/functional/tools/samples_tool_conf.xml | 3 + test/unit/jobs/test_job_wrapper.py | 2 +- test/unit/tools/test_toolbox.py | 49 ++++++++++++- test/unit/tools_support.py | 23 ++++-- test/unit/workflows/workflow_support.py | 2 +- 17 files changed, 287 insertions(+), 41 deletions(-) create mode 100644 lib/galaxy/tools/toolbox/lineages/stock.py create mode 100644 test/functional/tools/multiple_versions_v01.xml create mode 100644 test/functional/tools/multiple_versions_v02.xml diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 677f5b69a30..6c98ca43bca 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -711,7 +711,7 @@ class JobWrapper( object ): self.job_id = job.id self.session_id = job.session_id self.user_id = job.user_id - self.tool = queue.app.toolbox.get_tool( job.tool_id, exact=True ) + self.tool = queue.app.toolbox.get_tool( job.tool_id, job.tool_version, exact=True ) self.queue = queue self.app = queue.app self.sa_session = self.app.model.context diff --git a/lib/galaxy/jobs/handler.py b/lib/galaxy/jobs/handler.py index 07a859d7f16..9504a26dbc3 100644 --- a/lib/galaxy/jobs/handler.py +++ b/lib/galaxy/jobs/handler.py @@ -119,7 +119,7 @@ class JobHandlerQueue( object ): & ( model.Job.handler == self.app.config.server_name ) ).all() for job in jobs_at_startup: - if not self.app.toolbox.has_tool( job.tool_id, exact=True ): + if not self.app.toolbox.has_tool( job.tool_id, job.tool_version, exact=True ): log.warning( "(%s) Tool '%s' removed from tool config, unable to recover job" % ( job.id, job.tool_id ) ) self.job_wrapper( job ).fail( 'This tool was disabled before the job completed. Please contact your Galaxy administrator.' ) elif job.job_runner_name is not None and job.job_runner_external_id is None: diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 964443a726d..b7fb93cb653 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -126,6 +126,10 @@ class ToolBox( object, Dictifiable ): # shed_tool_conf.xml file. self._dynamic_tool_confs = [] self._tools_by_id = {} + # Tool lineages can contain chains of related tools with different ids + # so each will be present once in the above dictionary. The following + # dictionary can instead hold multiple tools with different versions. + self._tool_versions_by_id = {} self._workflows_by_id = {} # In-memory dictionary that defines the layout of the tool panel. self._tool_panel = odict() @@ -352,7 +356,8 @@ class ToolBox( object, Dictifiable ): inserted = True if not inserted: # Check the tool's installed versions. - for lineage_id in tool.lineage_ids: + for tool_lineage_version in tool.lineage.get_versions(): + lineage_id = tool_lineage_version.id lineage_id_key = 'tool_%s' % lineage_id for index, integrated_panel_key in enumerate( self._integrated_tool_panel.keys() ): if lineage_id_key == integrated_panel_key: @@ -503,20 +508,26 @@ class ToolBox( object, Dictifiable ): def get_tool( self, tool_id, tool_version=None, get_all_versions=False, exact=False ): """Attempt to locate a tool in the tool box.""" + if tool_version: + tool_version = str( tool_version ) + if get_all_versions and exact: raise AssertionError("Cannot specify get_tool with both get_all_versions and exact as True") if tool_id in self._tools_by_id and not get_all_versions: + if tool_version and tool_version in self._tool_versions_by_id[ tool_id ]: + return self._tool_versions_by_id[ tool_id ][ tool_version ] #tool_id exactly matches an available tool by id (which is 'old' tool_id or guid) return self._tools_by_id[ tool_id ] #exact tool id match not found, or all versions requested, search for other options, e.g. migrated tools or different versions rval = [] tool_lineage = self._lineage_map.get( tool_id ) if tool_lineage: - tool_version_ids = tool_lineage.get_version_ids( ) - for tool_version_id in tool_version_ids: - if tool_version_id in self._tools_by_id: - rval.append( self._tools_by_id[ tool_version_id ] ) + lineage_tool_versions = tool_lineage.get_versions( ) + for lineage_tool_version in lineage_tool_versions: + lineage_tool = self._tool_from_lineage_version( lineage_tool_version ) + if lineage_tool: + rval.append( lineage_tool ) if not rval: #still no tool, do a deeper search and try to match by old ids for tool in self._tools_by_id.itervalues(): @@ -561,11 +572,12 @@ class ToolBox( object, Dictifiable ): """Get all loaded tools associated by lineage to the tool whose id is tool_id.""" tool_lineage = self._lineage_map.get( tool_id ) if tool_lineage: - tool_version_ids = tool_lineage.get_version_ids( ) + lineage_tool_versions = tool_lineage.get_versions( ) available_tool_versions = [] - for tool_version_id in tool_version_ids: - if tool_version_id in self._tools_by_id: - available_tool_versions.append( self._tools_by_id[ tool_version_id ] ) + for lineage_tool_version in lineage_tool_versions: + tool = self._tool_from_lineage_version( lineage_tool_version ) + if tool: + available_tool_versions.append( tool ) return available_tool_versions else: if tool_id in self._tools_by_id: @@ -746,7 +758,6 @@ class ToolBox( object, Dictifiable ): tool_lineage = self._lineage_map.register( tool, tool_shed_repository=tool_shed_repository ) # Load the tool's lineage ids. tool.lineage = tool_lineage - tool.lineage_ids = tool_lineage.get_version_ids( ) self._tool_tag_manager.handle_tags( tool.id, elem ) self.__add_tool( tool, load_panel_dict, panel_dict ) # Always load the tool into the integrated_panel_dict, or it will not be included in the integrated_tool_panel.xml file. @@ -925,7 +936,20 @@ class ToolBox( object, Dictifiable ): return tool def register_tool( self, tool ): - self._tools_by_id[ tool.id ] = tool + tool_id = tool.id + version = tool.version or None + if tool_id not in self._tool_versions_by_id: + self._tool_versions_by_id[ tool_id ] = { version: tool } + else: + self._tool_versions_by_id[ tool_id ][ version ] = tool + if tool_id in self._tools_by_id: + related_tool = self._tools_by_id[ tool_id ] + # This one becomes the default un-versioned tool + # if newer. + if self._newer_tool( tool, related_tool ): + self._tools_by_id[ tool_id ] = tool + else: + self._tools_by_id[ tool_id ] = tool def package_tool( self, trans, tool_id ): """ @@ -1202,9 +1226,11 @@ class ToolBox( object, Dictifiable ): if not hasattr( tool, "lineage" ): return None tool_lineage = tool.lineage - lineage_ids = tool_lineage.get_version_ids( reverse=True ) - for lineage_id in lineage_ids: - if lineage_id in self._tools_by_id: + lineage_tool_versions = tool_lineage.get_versions( reverse=True ) + for lineage_tool_version in lineage_tool_versions: + lineage_tool = self._tool_from_lineage_version( lineage_tool_version ) + if lineage_tool: + lineage_id = lineage_tool.id loaded_version_key = 'tool_%s' % lineage_id if loaded_version_key in panel_dict: return panel_dict[ loaded_version_key ] @@ -1214,7 +1240,22 @@ class ToolBox( object, Dictifiable ): """ Return True if tool1 is considered "newer" given its own lineage description. """ - return tool1.lineage_ids.index( tool1.id ) > tool1.lineage_ids.index( tool2.id ) + if not hasattr( tool1, "lineage" ): + return True + lineage_tool_versions = tool1.lineage.get_versions() + for lineage_tool_version in lineage_tool_versions: + lineage_tool = self._tool_from_lineage_version( lineage_tool_version ) + if lineage_tool is tool1: + return False + if lineage_tool is tool2: + return True + return True + + def _tool_from_lineage_version( self, lineage_tool_version ): + if lineage_tool_version.id_based: + return self._tools_by_id.get( lineage_tool_version.id, None ) + else: + return self._tool_versions_by_id.get( lineage_tool_version.id, {} ).get( lineage_tool_version.version, None ) def _filter_for_panel( item, filters, context ): diff --git a/lib/galaxy/tools/toolbox/lineages/factory.py b/lib/galaxy/tools/toolbox/lineages/factory.py index 77efe944017..eb28ec06f11 100644 --- a/lib/galaxy/tools/toolbox/lineages/factory.py +++ b/lib/galaxy/tools/toolbox/lineages/factory.py @@ -1,4 +1,5 @@ from .tool_shed import ToolShedLineage +from .stock import StockLineage class LineageMap(object): @@ -13,7 +14,10 @@ class LineageMap(object): tool_id = tool.id if tool_id not in self.lineage_map: tool_shed_repository = kwds.get("tool_shed_repository", None) - lineage = ToolShedLineage.from_tool(self.app, tool, tool_shed_repository) + if tool_shed_repository: + lineage = ToolShedLineage.from_tool(self.app, tool, tool_shed_repository) + else: + lineage = StockLineage.from_tool( tool ) self.lineage_map[tool_id] = lineage return self.lineage_map[tool_id] diff --git a/lib/galaxy/tools/toolbox/lineages/interface.py b/lib/galaxy/tools/toolbox/lineages/interface.py index 1135213b8d1..46a49ecb87d 100644 --- a/lib/galaxy/tools/toolbox/lineages/interface.py +++ b/lib/galaxy/tools/toolbox/lineages/interface.py @@ -8,7 +8,35 @@ class ToolLineage(object): __metaclass__ = ABCMeta @abstractmethod - def get_version_ids( self, reverse=False ): - """ Return an ordered list of lineages in this chain, from - oldest to newest. + def get_versions( self, reverse=False ): + """ Return an ordered list of lineages (ToolLineageVersion) in this + chain, from oldest to newest. """ + + +class ToolLineageVersion(object): + """ Represents a single tool in a lineage. If lineage is based + around GUIDs that somehow encode the version (either using GUID + or a simple tool id and a version). """ + + def __init__(self, id, version): + self.id = id + self.version = version + + @staticmethod + def from_id_and_verion( id, version ): + assert version is not None + return ToolLineageVersion( id, version ) + + @staticmethod + def from_guid( guid ): + return ToolLineageVersion( guid, None ) + + @property + def id_based( self ): + """ Return True if the lineage is defined by GUIDs (in this + case the indexer of the tools (i.e. the ToolBox) should ignore + the tool_version (because it is encoded in the GUID and managed + externally). + """ + return self.version is None diff --git a/lib/galaxy/tools/toolbox/lineages/stock.py b/lib/galaxy/tools/toolbox/lineages/stock.py new file mode 100644 index 00000000000..c2db57bee7f --- /dev/null +++ b/lib/galaxy/tools/toolbox/lineages/stock.py @@ -0,0 +1,44 @@ +import threading + +from distutils.version import LooseVersion + +from .interface import ToolLineage +from .interface import ToolLineageVersion + + +class StockLineage(ToolLineage): + """ Simple tool's loaded directly from file system with lineage + determined solely by distutil's LooseVersion naming scheme. + """ + lineages_by_id = {} + lock = threading.Lock() + + def __init__(self, tool_id, **kwds): + self.tool_id = tool_id + self.tool_versions = set() + + @staticmethod + def from_tool( tool ): + tool_id = tool.id + lineages_by_id = StockLineage.lineages_by_id + with StockLineage.lock: + if tool_id not in lineages_by_id: + lineages_by_id[ tool_id ] = StockLineage( tool_id ) + lineage = lineages_by_id[ tool_id ] + lineage.register_version( tool.version ) + return lineage + + def register_version( self, tool_version ): + assert tool_version is not None + self.tool_versions.add( tool_version ) + + def get_versions( self, reverse=False ): + versions = [ ToolLineageVersion( self.tool_id, v ) for v in self.tool_versions ] + # Sort using LooseVersion which defines an appropriate __cmp__ + # method for comparing tool versions. + return sorted( versions, key=_to_loose_version ) + + +def _to_loose_version( tool_lineage_version ): + version = str( tool_lineage_version.version ) + return LooseVersion( version ) diff --git a/lib/galaxy/tools/toolbox/lineages/tool_shed.py b/lib/galaxy/tools/toolbox/lineages/tool_shed.py index 66b83e05798..7e261c367fb 100644 --- a/lib/galaxy/tools/toolbox/lineages/tool_shed.py +++ b/lib/galaxy/tools/toolbox/lineages/tool_shed.py @@ -1,4 +1,5 @@ from .interface import ToolLineage +from .interface import ToolLineageVersion from galaxy.model.tool_shed_install import ToolVersion @@ -32,6 +33,9 @@ class ToolShedLineage(ToolLineage): tool_version = self.app.install_model.context.query( ToolVersion ).get( self.tool_version_id ) return tool_version.get_version_ids( self.app, reverse=reverse ) + def get_versions( self, reverse=False ): + return map( ToolLineageVersion.from_guid, self.get_version_ids( reverse=reverse ) ) + def get_install_tool_version( app, tool_id ): return app.install_model.context.query( diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 253505f7dd9..c46536e27b2 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -467,10 +467,10 @@ class ToolModule( WorkflowModule ): type = "tool" - def __init__( self, trans, tool_id ): + def __init__( self, trans, tool_id, tool_version=None ): self.trans = trans self.tool_id = tool_id - self.tool = trans.app.toolbox.get_tool( tool_id ) + self.tool = trans.app.toolbox.get_tool( tool_id, tool_version=tool_version ) self.post_job_actions = {} self.workflow_outputs = [] self.state = None @@ -493,11 +493,14 @@ class ToolModule( WorkflowModule ): @classmethod def from_dict( Class, trans, d, secure=True ): tool_id = d[ 'tool_id' ] - module = Class( trans, tool_id ) + tool_version = str( d.get( 'tool_version', None ) ) + module = Class( trans, tool_id, tool_version=tool_version ) module.state = galaxy.tools.DefaultToolState() if module.tool is not None: if d.get('tool_version', 'Unspecified') != module.get_tool_version(): - module.version_changes.append( "%s: using version '%s' instead of version '%s' indicated in this workflow." % ( tool_id, d.get( 'tool_version', 'Unspecified' ), module.get_tool_version() ) ) + message = "%s: using version '%s' instead of version '%s' indicated in this workflow." % ( tool_id, d.get( 'tool_version', 'Unspecified' ), module.get_tool_version() ) + log.debug(message) + module.version_changes.append(message) module.state.decode( d[ "tool_state" ], module.tool, module.trans.app, secure=secure ) module.errors = d.get( "tool_errors", None ) module.post_job_actions = d.get( "post_job_actions", {} ) @@ -519,9 +522,12 @@ class ToolModule( WorkflowModule ): # This step has its state saved in the config field due to the # tool being previously unavailable. return module_factory.from_dict(trans, loads(step.config), secure=False) - module = Class( trans, tool_id ) + tool_version = step.tool_version + module = Class( trans, tool_id, tool_version=tool_version ) if step.tool_version and (step.tool_version != module.tool.version): - module.version_changes.append("%s: using version '%s' instead of version '%s' indicated in this workflow." % (tool_id, module.tool.version, step.tool_version)) + message = "%s: using version '%s' instead of version '%s' indicated in this workflow." % (tool_id, module.tool.version, step.tool_version) + log.debug(message) + module.version_changes.append(message) module.recover_state( step.tool_inputs ) module.errors = step.tool_errors module.workflow_outputs = step.workflow_outputs @@ -723,7 +729,7 @@ class ToolModule( WorkflowModule ): return state, step_errors def execute( self, trans, progress, invocation, step ): - tool = trans.app.toolbox.get_tool( step.tool_id ) + tool = trans.app.toolbox.get_tool( step.tool_id, tool_version=step.tool_version ) tool_state = step.state collections_to_match = self._find_collections_to_match( tool, progress, step ) diff --git a/test/api/test_tools.py b/test/api/test_tools.py index 6d02738796c..914ff517de4 100644 --- a/test/api/test_tools.py +++ b/test/api/test_tools.py @@ -165,6 +165,18 @@ class ToolsTestCase( api.ApiTestCase ): output1_content = self.dataset_populator.get_history_dataset_content( history_id, dataset=output1 ) self.assertEqual( output1_content.strip(), "Cat1Testlistified" ) + @skip_without_tool( "multiple_versions" ) + def test_run_by_versions( self ): + for version in ["0.1", "0.2"]: + # Run simple non-upload tool with an input data parameter. + history_id = self.dataset_populator.new_history() + inputs = dict() + outputs = self._run_and_get_outputs( tool_id="multiple_versions", history_id=history_id, inputs=inputs, tool_version=version ) + self.assertEquals( len( outputs ), 1 ) + output1 = outputs[ 0 ] + output1_content = self.dataset_populator.get_history_dataset_content( history_id, dataset=output1 ) + self.assertEqual( output1_content.strip(), "Version " + version ) + @skip_without_tool( "cat1" ) def test_run_cat1_single_meta_wrapper( self ): # Wrap input in a no-op meta parameter wrapper like Sam is planning to @@ -650,8 +662,8 @@ class ToolsTestCase( api.ApiTestCase ): def _cat1_outputs( self, history_id, inputs ): return self._run_outputs( self._run_cat1( history_id, inputs ) ) - def _run_and_get_outputs( self, tool_id, history_id, inputs ): - return self._run_outputs( self._run( tool_id, history_id, inputs ) ) + def _run_and_get_outputs( self, tool_id, history_id, inputs, tool_version=None ): + return self._run_outputs( self._run( tool_id, history_id, inputs, tool_version=tool_version ) ) def _run_outputs( self, create_response ): self._assert_status_code_is( create_response, 200 ) @@ -660,12 +672,14 @@ class ToolsTestCase( api.ApiTestCase ): def _run_cat1( self, history_id, inputs, assert_ok=False ): return self._run( 'cat1', history_id, inputs, assert_ok=assert_ok ) - def _run( self, tool_id, history_id, inputs, assert_ok=False ): + def _run( self, tool_id, history_id, inputs, assert_ok=False, tool_version=None ): payload = self.dataset_populator.run_tool_payload( tool_id=tool_id, inputs=inputs, history_id=history_id, ) + if tool_version is not None: + payload[ "tool_version" ] = tool_version create_response = self._post( "tools", data=payload ) if assert_ok: self._assert_status_code_is( create_response, 200 ) diff --git a/test/api/test_workflows.py b/test/api/test_workflows.py index b092a0284f5..696c2d722a7 100644 --- a/test/api/test_workflows.py +++ b/test/api/test_workflows.py @@ -444,6 +444,28 @@ class WorkflowsApiTestCase( BaseWorkflowsApiTestCase ): def test_run_workflow( self ): self.__run_cat_workflow( inputs_by='step_id' ) + @skip_without_tool( "multiple_versions" ) + def test_run_versioned_tools( self ): + history_01_id = self.dataset_populator.new_history() + workflow_version_01 = self._upload_yaml_workflow( """ +- tool_id: multiple_versions + tool_version: "0.1" + state: + inttest: 0 +""" ) + self.__invoke_workflow( history_01_id, workflow_version_01 ) + self.dataset_populator.wait_for_history( history_01_id, assert_ok=True ) + + history_02_id = self.dataset_populator.new_history() + workflow_version_02 = self._upload_yaml_workflow( """ +- tool_id: multiple_versions + tool_version: "0.2" + state: + inttest: 1 +""" ) + self.__invoke_workflow( history_02_id, workflow_version_02 ) + self.dataset_populator.wait_for_history( history_02_id, assert_ok=True ) + def __run_cat_workflow( self, inputs_by ): workflow = self.workflow_populator.load_workflow( name="test_for_run" ) workflow["steps"]["0"]["uuid"] = str(uuid4()) @@ -910,7 +932,7 @@ test_data: self._assert_status_code_is( hda_info_response, 200 ) self.assertEquals( hda_info_response.json()[ "metadata_data_lines" ], lines ) - def __invoke_workflow( self, history_id, workflow_id, inputs, assert_ok=True ): + def __invoke_workflow( self, history_id, workflow_id, inputs={}, assert_ok=True ): workflow_request = dict( history="hist_id=%s" % history_id, ) diff --git a/test/functional/tools/multiple_versions_v01.xml b/test/functional/tools/multiple_versions_v01.xml new file mode 100644 index 00000000000..8b05454529f --- /dev/null +++ b/test/functional/tools/multiple_versions_v01.xml @@ -0,0 +1,11 @@ + + + echo "Version 0.1" > $out_file1 + + + + + + + + diff --git a/test/functional/tools/multiple_versions_v02.xml b/test/functional/tools/multiple_versions_v02.xml new file mode 100644 index 00000000000..7a7a3a14937 --- /dev/null +++ b/test/functional/tools/multiple_versions_v02.xml @@ -0,0 +1,11 @@ + + + echo "Version 0.2" > $out_file1 + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 751eb627264..d0a1f8d1974 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -36,6 +36,9 @@ + + +