From 113bff99bc744ef237a52f49e9e7fea6ce261776 Mon Sep 17 00:00:00 2001 From: James Taylor Date: Thu, 8 Mar 2007 17:52:29 +0000 Subject: [PATCH] Use an association object for the relationship between parent and child datasets. THIS REQUIRES DATABASE CHANGES! Also, test parse errors should now cause errors. --- lib/galaxy/interfaces/root.py | 30 +++--- lib/galaxy/jobs/__init__.py | 14 +-- lib/galaxy/model/__init__.py | 19 +++- lib/galaxy/model/mapping.py | 17 ++- lib/galaxy/tools/__init__.py | 185 +++++++++++++++++++------------- lib/galaxy/tools/test.py | 11 +- templates/history.tmpl | 34 +++--- test/functional/test_toolbox.py | 23 +++- 8 files changed, 208 insertions(+), 125 deletions(-) diff --git a/lib/galaxy/interfaces/root.py b/lib/galaxy/interfaces/root.py index 076ca32d27c..b4618625b46 100644 --- a/lib/galaxy/interfaces/root.py +++ b/lib/galaxy/interfaces/root.py @@ -202,8 +202,8 @@ class Universe(common.Root): data = self.app.model.Dataset.get( id ) if data: assert data in history.datasets, "Data does not belong to current history" - assert data.parent == None, "You must delete the primary dataset first." - history.datasets.remove( data ) + # assert data.parent == None, "You must delete the primary dataset first." + # history.datasets.remove( data ) data.deleted = True trans.log_event( "Dataset id %s marked as deleted" % str(id) ) self.app.model.flush() @@ -216,8 +216,8 @@ class Universe(common.Root): data = self.app.model.Dataset.get( id ) if data: assert data in history.datasets, "Data does not belong to current history" - assert data.parent == None, "You must delete the primary dataset first." - history.datasets.remove( data ) + # assert data.parent == None, "You must delete the primary dataset first." + # history.datasets.remove( data ) data.deleted = True trans.log_event( "Dataset id %s marked as deleted async" % str(id) ) self.app.model.flush() @@ -527,7 +527,7 @@ class Universe(common.Root): des.state = src.state des.metadata = src.metadata des.hid = src.hid - des.parent_id = parent_id + ## des.parent_id = parent_id shutil.copyfile(src.file_name,des.file_name) des.hid = src.hid des.designation = src.designation @@ -540,15 +540,17 @@ class Universe(common.Root): des.name = src.name des.user_id = src.user_id for data in src.datasets: - if not data.parent: - new_data = self.copy_dataset(data) - des.add_dataset(new_data) - new_data.hid = data.hid - new_data.flush() - for child in data.children: - new_child = self.copy_dataset(child, parent_id=new_data.id) - des.add_dataset(new_child, parent_id = new_data.id) - new_child.flush() + new_data = self.copy_dataset(data) + des.add_dataset(new_data) + new_data.hid = data.hid + new_data.flush() + for child_assoc in data.children: + new_child = self.copy_dataset(child_assoc.child) + new_assoc = self.app.model.DatasetAssociation( child.designation ) + new_assoc.child = new_child + new_assoc.parent = new_data + #des.add_dataset(new_child, parent_id = new_data.id) + new_child.flush() des.hid_counter = src.hid_counter self.app.model.flush() return des \ No newline at end of file diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 4df8624c36b..f58ca969380 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -237,12 +237,14 @@ class JobWrapper( object ): errors = out_dataset.dataset.datatype.validate(out_dataset.dataset) if len(errors) > 0: # create child dataset with errors - errordata = self.app.model.Dataset(hid=out_dataset.dataset.hid, - parent_id=out_dataset.dataset.id, - state=model.Dataset.states.OK, - extension='text', - name='Errors', - info='', blurb='Errors!') + errordata = model.Dataset( hid=out_dataset.dataset.hid, + state=model.Dataset.states.OK, + extension='text', + name='Errors', + info='', blurb='Errors!') + assoc = model.DatasetChildAssociation() + assoc.child = errordata + out_dataset.children.append( assoc ) errordata.flush() errorfile = open(errordata.file_name,"w") for error in errors: diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 56604d2e90e..2313b6c5544 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -240,11 +240,14 @@ class Dataset( object ): def get_ucsc_sites( self ): return self.datatype.get_ucsc_sites( self ) def get_child_by_designation(self, designation): - if self.history: - for data in self.history.datasets: - if data.parent_id and data.parent_id == self.id: - if designation == data.designation: - return data + # if self.history: + # for data in self.history.datasets: + # if data.parent_id and data.parent_id == self.id: + # if designation == data.designation: + # return data + for child_assocation in self.children: + if child_association.designation == designation: + return child return None # FIXME: sqlalchemy will replace this def _delete(self): @@ -254,6 +257,12 @@ class Dataset( object ): except OSError, e: log.critical('%s delete error %s' % (self.__class__.__name__, e)) +class DatasetChildAssociation( object ): + def __init__( self, designation=None ): + self.designation = designation + self.parent = None + self.child = None + class Event( object ): def __init__( self, message=None, history=None, user=None, galaxy_session=None ): self.history = history diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index cae1d7d26fa..1fad4c148ed 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -4,7 +4,7 @@ are encapsulated here. """ import pkg_resources -pkg_resources.require( "pysqlite>=2", "sqlalchemy>=0.3" ) +pkg_resources.require( "psycopg2", "pysqlite>=2", "sqlalchemy>=0.3" ) import sys @@ -69,6 +69,12 @@ Dataset.table = Table( "dataset", metadata, Column( "deleted", Boolean ), ForeignKeyConstraint(['parent_id'],['dataset.id'], ondelete="CASCADE") ) +DatasetChildAssociation.table = Table( "dataset_child_association", metadata, + Column( "id", Integer, primary_key=True ), + Column( "parent_dataset_id", Integer, ForeignKey( "dataset.id" ) ), + Column( "child_dataset_id", Integer, ForeignKey( "dataset.id" ) ), + Column( "designation", TrimmedString( 255 ) ) ) + Job.table = Table( "job", metadata, Column( "id", Integer, primary_key=True ), Column( "create_time", DateTime, PassiveDefault( func.current_timestamp() ) ), @@ -129,8 +135,11 @@ GalaxySessionToHistoryAssociation.table = Table( "galaxy_session_to_history", me # relationships between the model objects. assign_mapper( context, Dataset, Dataset.table, - properties=dict( children=relation( Dataset, cascade="delete", - backref=backref("parent", foreignkey=Dataset.table.c.id ) ) ) ) + properties=dict( children=relation( DatasetChildAssociation, primaryjoin=( DatasetChildAssociation.table.c.parent_dataset_id == Dataset.table.c.id ), + lazy=False ) ) ) + +assign_mapper( context, DatasetChildAssociation, DatasetChildAssociation.table, + properties=dict( child=relation( Dataset, primaryjoin=( DatasetChildAssociation.table.c.child_dataset_id == Dataset.table.c.id ) ) ) ) # assign_mapper( model.Query, model.Query.table, # properties=dict( datasets=relation( model.Dataset.mapper, backref="query") ) ) @@ -199,7 +208,7 @@ def init( file_path, url, **kwargs ): Dataset.file_path = file_path # Connect the metadata the database. metadata.connect( url, **kwargs ) - ##metadata.engine.echo = True + metadata.engine.echo = True # Create tables if needed if create_tables: metadata.create_all() diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 2ff901772ae..baf8829599b 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -8,7 +8,7 @@ from cookbook.patterns import Bunch from galaxy import util, jobs from elementtree import ElementTree from parameters import * -from galaxy.tools.test import ToolTestBuilder +from galaxy.tools.test import ToolTestBuilder, BadToolTest log = logging.getLogger( __name__ ) @@ -245,20 +245,29 @@ class Tool: self.tests = [] for i, test_elem in enumerate( tests_elem.findall( 'test' ) ): name = test_elem.get( 'name', 'Test-%d' % (i+1) ) - test = ToolTestBuilder( self, name ) - for param_elem in test_elem.findall( "param" ): - attrib = dict( param_elem.attrib ) - if 'values' in attrib: - value = attrib[ 'values' ].split( ',' ) - elif 'value' in attrib: - value = attrib['value'] - else: - value = None - test.add_param( attrib.pop( 'name' ), value, attrib ) - for output_elem in test_elem.findall( "output" ): - attrib = dict( output_elem.attrib ) - test.add_output( attrib.pop( 'name' ), attrib.pop( 'file' ) ) - self.tests.append( test ) + try: + test = ToolTestBuilder( self, name ) + for param_elem in test_elem.findall( "param" ): + attrib = dict( param_elem.attrib ) + if 'values' in attrib: + value = attrib[ 'values' ].split( ',' ) + elif 'value' in attrib: + value = attrib['value'] + else: + value = None + test.add_param( attrib.pop( 'name' ), value, attrib ) + for output_elem in test_elem.findall( "output" ): + attrib = dict( output_elem.attrib ) + name = attrib.pop( 'name', None ) + if name is None: + raise Exception( "Test output does not have a 'name'" ) + file = attrib.pop( 'file', None ) + if file is None: + raise Exception( "Test output does not have a 'file'") + test.add_output( name, file ) + self.tests.append( test ) + except Exception, e: + self.tests.append( BadToolTest( self, name, e ) ) def parse_page( self, input_elem, enctypes ): param_map = odict() @@ -407,12 +416,14 @@ class Tool: # $dataset.get_child( 'name' ).filename for name, data in input_datasets.items(): param_dict[name] = DatasetFilenameWrapper( data ) - for child in data.children: + for child_association in data.children: + child = child_association.child key = "_CHILD___%s___%s" % ( name, child.designation ) param_dict[ key ] = DatasetFilenameWrapper( child ) for name, data in output_datasets.items(): param_dict[name] = DatasetFilenameWrapper( data ) - for child in data.children: + for child_association in data.children: + child = child_association.child key = "_CHILD___%s___%s" % ( name, child.designation ) param_dict[ key ] = DatasetFilenameWrapper( child ) # Return the dictionary of parameters @@ -478,34 +489,48 @@ class DefaultToolAction( object ): """ Default tool action is to run an external command """ + + def collect_input_datasets( self, tool, incoming ): + """ + Collect any dataset inputs from incoming. Returns a mapping from + parameter name to Dataset instance for each tool parameter that is + of the DataToolParameter type. + """ + input_datasets = dict() + for name, value in incoming.iteritems(): + param = tool.get_param( name ) + if param and isinstance( param, DataToolParameter ): + if isinstance( value, list ): + # If there are multiple inputs with the same name, they + # are stored as name1, name2, ... + for i, v in enumerate( value ): + input_datasets[ name + str( i + 1 ) ] = v + else: + input_datasets[ name ] = value + return input_datasets + def execute(self, tool, trans, incoming={} ): - inp_data = {} out_data = {} - # collect the input data - for name, value in incoming.items(): - param = tool.get_param(name) - if param: - count = 1 - if isinstance(param, DataToolParameter ): - # multiple inputs on the same parameter name will be created as name1, name2 ... - if isinstance( value, list ): - for v in value: - inp_data[name+str(count)] = v - count += 1 - else: - inp_data[name] = value - # input metadata + # Collect any input datasets from the incoming parameters + inp_data = self.collect_input_datasets( tool, incoming ) + + # Deal with input metadata, 'dbkey', names, and types + # FIXME: does this need to modify 'incoming' or should this be - # moved into 'build_param_dict'? - input_names, input_ext, input_dbkey, input_meta = [ ], 'data', incoming.get("dbkey", "?"), Bunch() + # moved into 'build_param_dict'? Is this just about getting the + # metadata into the command line? + input_names = [] + input_ext = 'data' + input_dbkey = incoming.get( "dbkey", "?" ) + input_meta = Bunch() for name, data in inp_data.items(): - #fix for fake incoming data + # Hack for fake incoming data if data == None: data = trans.app.model.Dataset() data.state = data.states.FAKE - input_names.append( 'data %s' % data.hid) - input_ext = data.ext + input_names.append( 'data %s' % data.hid ) + input_ext = data.ext if data.dbkey not in [None, '?']: input_dbkey = data.dbkey for meta_key, meta_value in data.metadata.items(): @@ -513,74 +538,84 @@ class DefaultToolAction( object ): meta_key = '%s_%s' % (name, meta_key) incoming[meta_key] = meta_value - # format input names for display - if input_names: - input_names = 'on ' + ', '.join(input_names) - else: - input_names = '' + # Build name for output datasets based on tool name and input names + output_base_name = tool.name + if input_names: + output_base_name += ' on ' + ', '.join( input_names ) - # add the dbkey to the incoming parameters - incoming["dbkey"] = input_dbkey + # Add the dbkey to the incoming parameters + incoming[ "dbkey" ] = input_dbkey + + # Keep track of parent / child relationships, we'll create all the + # datasets first, then create the associations + parent_to_child_pairs = [] + child_dataset_names = set() - #Use to store param_name -> data id and existing child/parent relationships - name_id = {} - child_parent = {} - # create the output data for name, elems in tool.outputs.items(): - (ext, metadata_source, parent) = elems + ( ext, metadata_source, parent ) = elems if parent: - child_parent[name]=parent - # hack! the output data has already been created + parent_to_child_pairs.append( ( parent, name ) ) + child_dataset_names.add( name ) + ## What is the following hack for? Need to document under what + ## conditions can the following occur? (james@bx.psu.edu) + # HACK: the output data has already been created if name in incoming: dataid = incoming[name] data = trans.app.model.Dataset.get( dataid ) assert data != None out_data[name] = data continue - # the type should match the input if ext == "input": ext = input_ext - + # FIXME: What does this flush? trans.app.model.flush() data = trans.app.model.Dataset() - # Commit immediately so it gets an id + # Commit the dataset immediately so it gets database assigned + # unique id data.flush() - - #create an empty file - open(data.file_name,"w").close() - + # Create an empty file immediately + open( data.file_name, "w" ).close() + # FIXME: What does this flush? trans.app.model.flush() - + # This may not be neccesary with the new parent/child associations data.designation = name - + # Set the extension / datatype + # FIXME: Datatypes -- this propertype has a lot of hidden logic data.extension = ext + # Copy metadata from one of the inputs if requested. + # FIXME: init_meta should take a dataset to copy from as an + # argument if metadata_source: data.metadata = Bunch( ** inp_data[metadata_source].metadata.__dict__ ) else: data.init_meta() - + # Take dbkey from LAST input data.dbkey = input_dbkey + # Default attributes data.state = data.states.QUEUED data.blurb = "queued" - data.name = '%s %s' % (tool.name, input_names) - out_data[name] = data + data.name = output_base_name + out_data[ name ] = data + # Store all changes to database trans.app.model.flush() - name_id[name] = data.id - - #add parent datasets to history + + # Add all the top-level (non-child) datasets to the history for name in out_data.keys(): - if name not in child_parent.keys(): - data = out_data[name] + if name not in child_dataset_names: + data = out_data[ name ] trans.history.add_dataset( data ) data.flush() - #add children datasets to history - for name in out_data.keys(): - if name in child_parent.keys(): - data = out_data[name] - data.parent_id = name_id[child_parent[name]] - trans.history.add_dataset( data, parent_id=data.parent_id ) - data.flush() + + # Add all the children to their parents + for parent_name, child_name in parent_to_child_pairs: + parent_dataset = out_data[ parent_name ] + child_dataset = out_data[ child_name ] + assoc = trans.app.model.DatasetChildAssociation() + assoc.child = child_dataset + assoc.designation = child_dataset.designation + parent_dataset.children.append( assoc ) + # FIXME: Child dataset hid # Store data after custom code runs trans.app.model.flush() diff --git a/lib/galaxy/tools/test.py b/lib/galaxy/tools/test.py index ec353e8228e..b85c31d2edf 100644 --- a/lib/galaxy/tools/test.py +++ b/lib/galaxy/tools/test.py @@ -19,4 +19,13 @@ class ToolTestBuilder( object ): self.required_files.append( ( value, extra ) ) self.inputs.append( ( name, value, extra ) ) def add_output( self, name, file ): - self.outputs.append( ( name, file ) ) \ No newline at end of file + self.outputs.append( ( name, file ) ) + +class BadToolTest( object ): + """ + Represents a tool test that failed to parse + """ + def __init__( self, tool, name, exception=None ): + self.tool = tool + self.name = name + self.exception = exception \ No newline at end of file diff --git a/templates/history.tmpl b/templates/history.tmpl index d109f8a4db6..84553f332ed 100644 --- a/templates/history.tmpl +++ b/templates/history.tmpl @@ -1,6 +1,6 @@ -#set $refresh= bool ( [ data for data in $history.active_datasets if data.state in ['running', 'queued','',None] ] ) +#set $refresh = bool ( [ data for data in $history.active_datasets if data.state in ['running', 'queued', '', None ] ] ) @@ -136,9 +136,6 @@ main();"> #else #for $data in reversed($history.active_datasets) - #if $data.parent - #continue - #end if
#if $data.state in ["no state","",None]: #set $data_state = "queued" @@ -152,11 +149,11 @@ main();"> - #if $data_state == 'running' -
- #elif $data_state == 'ok' - #pass - #else + #if $data_state == 'running' +
+ #elif $data_state == 'ok' + #pass + #else #set $src = "/static/style/data_%s.png" % $data_state
#end if @@ -223,8 +220,8 @@ main();"> #if $len($data.children)>0
There are $len($data.children) secondary datasets. - #for $idx, $child in $enumerate($data.children) - + #for $idx, $child_assoc in $enumerate($data.children) + #set $child = $child_assoc.child
#if $child.state in ["no state","",None]: #set $data_state = "queued" @@ -238,13 +235,13 @@ main();"> - #if $data_state == 'running' -
- #elif $data_state == 'ok' - #pass - #else - #set $src = "/static/style/data_%s.png" % $data_state -
+ #if $data_state == 'running' +
+ #elif $data_state == 'ok' + #pass + #else + #set $src = "/static/style/data_%s.png" % $data_state +
#end if
@@ -316,7 +313,6 @@ main();">
#end for - #end if diff --git a/test/functional/test_toolbox.py b/test/functional/test_toolbox.py index 61aa22ae1f4..39cfde16f4a 100644 --- a/test/functional/test_toolbox.py +++ b/test/functional/test_toolbox.py @@ -31,6 +31,13 @@ class ToolTestCase( TwillTestCase ): self.check_data( file ) def shortDescription( self ): return self.name + +class BadToolTestCase( TwillTestCase ): + def do_it( self ): + if self.testdef.exception: + raise self.testdef.exception + else: + raise Exception( "Test parse failure" ) def get_testcase( testdef, name ): """ @@ -42,6 +49,17 @@ def get_testcase( testdef, name ): self.do_it() d = dict( testdef=testdef, test_tool=test_tool, name=name ) return new.classobj( n, s, d ) + +def get_badtestcase( testdef, name ): + """ + Dynamically generate a `BadToolTestCase` for `testdef` + """ + n = "GeneratedToolTestCase_" + testdef.tool.id.replace( ' ', '_' ) + s = ( BadToolTestCase, ) + def test_tool( self ): + self.do_it() + d = dict( testdef=testdef, test_tool=test_tool, name=name ) + return new.classobj( n, s, d ) def setup(): """ @@ -58,5 +76,8 @@ def setup(): if tool.tests: for k, testdef in enumerate( tool.tests ): name = "%s > %s > %s" % ( section.name, tool.name, testdef.name ) - testcase = get_testcase( testdef, name ) + if isinstance( testdef, BadToolTest ): + testcase = get_badtestcase( testdef, name ) + else: + testcase = get_testcase( testdef, name ) G[ 'testcase_%d_%d_%d' % ( i, j, k ) ] = testcase \ No newline at end of file