From 3d09481a0d894b146e17b9522e22c564f3c500ae Mon Sep 17 00:00:00 2001 From: James Taylor Date: Sun, 22 Mar 2009 08:29:23 -0400 Subject: [PATCH 1/7] Bug fix for dataset error page (and other pages using Cheetah): 'output_encoding' is not a valid argument for Cheetah templates. Not sure what the right way to do it is. Of course, we should just convert any remaining Cheetah templates to mako --- lib/galaxy/web/framework/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/web/framework/__init__.py b/lib/galaxy/web/framework/__init__.py index 2f07ca817bc..b09f0666271 100644 --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -528,8 +528,7 @@ class UniverseWebTransaction( base.DefaultWebTransaction ): return self.fill_template_mako( filename, **kwargs ) else: template = Template( file=os.path.join(self.app.config.template_path, filename), - searchList=[kwargs, self.template_context, dict(caller=self, t=self, h=webhelpers, util=util, request=self.request, response=self.response, app=self.app)], - output_encoding='utf-8' ) + searchList=[kwargs, self.template_context, dict(caller=self, t=self, h=webhelpers, util=util, request=self.request, response=self.response, app=self.app)] ) return str( template ) def fill_template_mako( self, filename, **kwargs ): template = self.webapp.mako_template_lookup.get_template( filename ) From e21a04611b7b8111681eb35f849c92e1c6da49e1 Mon Sep 17 00:00:00 2001 From: James Taylor Date: Sun, 22 Mar 2009 10:06:29 -0400 Subject: [PATCH 2/7] Improve error message when validation of parameters fails at runtime --- lib/galaxy/jobs/__init__.py | 13 ++++-- lib/galaxy/tools/__init__.py | 50 ++++++++++++++++++----- lib/galaxy/tools/parameters/validation.py | 4 ++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index e8b210301b8..a759de8ab76 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -257,7 +257,7 @@ class JobQueue( object ): return JOB_INPUT_DELETED # an error in the input data causes us to bail immediately elif idata.state == idata.states.ERROR: - job_wrapper.fail( "input data %d (file: %s) is in an error state" % ( idata.hid, idata.file_name ) ) + job_wrapper.fail( "input data %d is in error state" % ( idata.hid ) ) return JOB_INPUT_ERROR elif idata.state != idata.states.OK: # need to requeue @@ -384,10 +384,17 @@ class JobWrapper( object ): job.refresh() # if the job was deleted, don't fail it if not job.state == model.Job.states.DELETED: - # If the failure is due to a Galaxy framework exception, save the traceback - # Do this first in case we generate a traceback below + # Check if the failure is due to an exception if exception: + # Save the traceback immediately in case we generate another + # below job.traceback = traceback.format_exc() + # Get the exception and let the tool attempt to generate + # a better message + etype, evalue, tb = sys.exc_info() + m = self.tool.handle_job_failure_exception( evalue ) + if m: + message = m if self.app.config.outputs_to_working_directory: for dataset_path in self.get_output_fnames(): try: diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 60672531fea..c72e5fdf48b 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -18,6 +18,7 @@ from galaxy import util, jobs, model from elementtree import ElementTree from parameters import * from parameters.grouping import * +from parameters.validation import LateValidationError from galaxy.util.expressions import ExpressionContext from galaxy.tools.test import ToolTestBuilder from galaxy.tools.actions import DefaultToolAction @@ -1053,31 +1054,60 @@ class Tool: return self.handle_unvalidated_param_values_helper( self.inputs, input_values, app ) - def handle_unvalidated_param_values_helper( self, inputs, input_values, app, context=None ): + def handle_unvalidated_param_values_helper( self, inputs, input_values, app, context=None, prefix="" ): """ Recursive helper for `handle_unvalidated_param_values` """ context = ExpressionContext( input_values, context ) for input in inputs.itervalues(): if isinstance( input, Repeat ): - for d in input_values[ input.name ]: - self.handle_unvalidated_param_values_helper( input.inputs, d, app, context ) + for i, d in enumerate( input_values[ input.name ] ): + rep_prefix = prefix + "%s %d > " % ( input.title, i + 1 ) + self.handle_unvalidated_param_values_helper( input.inputs, d, app, context, rep_prefix ) elif isinstance( input, Conditional ): values = input_values[ input.name ] current = values["__current_case__"] - self.handle_unvalidated_param_values_helper( input.cases[current].inputs, values, app, context ) + # NOTE: The test param doesn't need to be checked since + # there would be no way to tell what case to use at + # workflow build time. However I'm not sure if we are + # actually preventing such a case explicately. + self.handle_unvalidated_param_values_helper( input.cases[current].inputs, values, app, context, prefix ) else: # Regular tool parameter value = input_values[ input.name ] if isinstance( value, UnvalidatedValue ): - if value.value is None: #if value.value is None, it could not have been submited via html form and therefore .from_html can't be guaranteed to work - value = None - else: - value = input.from_html( value.value, None, context ) - # Then do any further validation on the value - input.validate( value, None ) + try: + # Convert from html representation + if value.value is None: + # If value.value is None, it could not have been + # submited via html form and therefore .from_html + # can't be guaranteed to work + value = None + else: + value = input.from_html( value.value, None, context ) + # Do any further validation on the value + input.validate( value, None ) + except Exception, e: + # Wrap an re-raise any generated error so we can + # generate a more informative message + v = input.value_to_display_text( value, self.app ) + message = "Failed runtime validation of %s%s (%s)" \ + % ( prefix, input.label, e ) + raise LateValidationError( message ) input_values[ input.name ] = value + def handle_job_failure_exception( self, e ): + """ + Called by job.fail when an exception is generated to allow generation + of a better error message (returning None yields the default behavior) + """ + message = None + # If the exception was generated by late validation, use its error + # message (contains the parameter name and value) + if isinstance( e, LateValidationError ): + message = e.message + return message + def build_param_dict( self, incoming, input_datasets, output_datasets, output_paths ): """ Build the dictionary of parameters for substituting into the command diff --git a/lib/galaxy/tools/parameters/validation.py b/lib/galaxy/tools/parameters/validation.py index 443824e1dee..06e56dface4 100644 --- a/lib/galaxy/tools/parameters/validation.py +++ b/lib/galaxy/tools/parameters/validation.py @@ -8,6 +8,10 @@ from galaxy import model log = logging.getLogger( __name__ ) +class LateValidationError( Exception ): + def __init__( self, message ): + self.message = message + class Validator( object ): """ A validator checks that a value meets some conditions OR raises ValueError From 4dce57dc0456e6783b81c1ae38bf5ce169b4457b Mon Sep 17 00:00:00 2001 From: James Taylor Date: Sun, 22 Mar 2009 10:33:07 -0400 Subject: [PATCH 3/7] Workflow parameterization! - When building a workflow, most parameters can be switched to be set later ("at runtime"). The main (only?) exception is parameters which are used as the test for conditionals - When running a workflow, the user fills in the runtime parameters just like in any other tool form. - As a consequence, the run workflow form behaves more like a full tool form (automatic refreshes, validation, et cetera). - There is still a problem when paramters depend on a data input that is not created until later. These should probably fall back on late validation, but that requires a bit of cleanup to param depedencies. --- lib/galaxy/tools/__init__.py | 13 ++- lib/galaxy/tools/parameters/basic.py | 48 +++++----- lib/galaxy/web/controllers/workflow.py | 10 +-- lib/galaxy/workflow/modules.py | 35 ++++++-- static/scripts/galaxy.base.js | 9 +- static/scripts/galaxy.panels.js | 23 ----- templates/workflow/editor.mako | 25 ++++-- templates/workflow/editor_tool_form.mako | 52 ++++++++--- templates/workflow/run.mako | 106 ++++++++++++++--------- 9 files changed, 196 insertions(+), 125 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index c72e5fdf48b..75238d16e2d 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -841,7 +841,8 @@ class Tool: return 'message.mako', dict( message_type='error', message='Your upload was interrupted. If this was uninentional, please retry it.', refresh_frames=[], cont=None ) def update_state( self, trans, inputs, state, incoming, prefix="", context=None, - update_only=False, old_errors={}, changed_dependencies={} ): + update_only=False, old_errors={}, changed_dependencies={}, + item_callback=None ): """ Update the tool state in `state` using the user input in `incoming`. This is designed to be called recursively: `inputs` contains the @@ -895,7 +896,8 @@ class Tool: context=context, update_only=update_only, old_errors=rep_old_errors, - changed_dependencies=changed_dependencies ) + changed_dependencies=changed_dependencies, + item_callback=item_callback ) if rep_errors: any_group_errors = True group_errors.append( rep_errors ) @@ -952,7 +954,8 @@ class Tool: context=context, update_only=update_only, old_errors=group_old_errors, - changed_dependencies=changed_dependencies ) + changed_dependencies=changed_dependencies, + item_callback=item_callback ) if test_param_error: group_errors[ input.test_param.name ] = test_param_error if group_errors: @@ -1003,6 +1006,10 @@ class Tool: if not incoming_value_generated: incoming_value = get_incoming_value( incoming, key, None ) value, error = check_param( trans, input, incoming_value, context ) + # If a callback was provided, allow it to process the value + if item_callback: + old_value = state.get( input.name, None ) + value, error = item_callback( trans, key, input, value, error, old_value, context ) if input.dependent_params and state[ input.name ] != value: # We need to keep track of changed dependency parametrs ( parameters # that have dependent parameters whose options are dynamically generated ) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index baaa545ae07..9eef2f3c12e 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -94,12 +94,18 @@ class ToolParameter( object ): return value def value_to_basic( self, value, app ): + if isinstance( value, RuntimeValue ): + return { "__class__": "RuntimeValue" } return self.to_string( value, app ) def value_from_basic( self, value, app, ignore_errors=False ): # HACK: Some things don't deal with unicode well, psycopg problem? if type( value ) == unicode: value = str( value ) + # Handle Runtime values (valid for any parameter?) + if isinstance( value, dict ) and '__class__' in value and value['__class__'] == "RuntimeValue": + return RuntimeValue() + # Delegate to the 'to_python' method if ignore_errors: try: return self.to_python( value, app ) @@ -567,12 +573,11 @@ class SelectToolParameter( ToolParameter ): def value_to_basic( self, value, app ): if isinstance( value, UnvalidatedValue ): return { "__class__": "UnvalidatedValue", "value": value.value } - return value + return super( SelectToolParameter, self ).value_to_basic( value, app ) def value_from_basic( self, value, app, ignore_errors=False ): - if isinstance( value, dict ): - assert value["__class__"] == "UnvalidatedValue" + if isinstance( value, dict ) and value["__class__"] == "UnvalidatedValue": return UnvalidatedValue( value["value"] ) - return value + return super( SelectToolParameter, self ).value_from_basic( value, app ) def get_initial_value( self, trans, context ): # More working around dynamic options for workflow if self.is_dynamic and ( trans is None or trans.workflow_building_mode )\ @@ -953,15 +958,6 @@ class DrillDownSelectToolParameter( ToolParameter ): assert self.multiple, "Multiple values provided but parameter is not expecting multiple values" return self.separator.join( rval ) - def value_to_basic( self, value, app ): - if isinstance( value, UnvalidatedValue ): - return { "__class__": "UnvalidatedValue", "value": value.value } - return value - def value_from_basic( self, value, app, ignore_errors=False ): - if isinstance( value, dict ): - assert value["__class__"] == "UnvalidatedValue" - return UnvalidatedValue( value["value"] ) - return value def get_initial_value( self, trans, context ): def recurse_options( initial_values, options ): for option in options: @@ -1174,25 +1170,17 @@ class DataToolParameter( ToolParameter ): else: return trans.app.model.HistoryDatasetAssociation.get( value ) - def value_to_basic( self, value, app ): + def to_string( self, value, app ): if value is None or isinstance( value, str ): return value return value.id - def value_from_basic( self, value, app, ignore_errors=False ): - """ - Both of these values indicate that no dataset is selected. However, 'None' - indicates that the dataset is optional, while '' indicates that it is not. - """ + def to_python( self, value, app ): + # Both of these values indicate that no dataset is selected. However, 'None' + # indicates that the dataset is optional, while '' indicates that it is not. if value is None or value == '' or value == 'None': return value - try: - return app.model.HistoryDatasetAssociation.get( int( value ) ) - except: - if ignore_errors: - return value - else: - raise + return app.model.HistoryDatasetAssociation.get( int( value ) ) def to_param_dict_string( self, value, other_values={} ): if value is None: return "None" @@ -1294,6 +1282,14 @@ class UnvalidatedValue( object ): """ def __init__( self, value ): self.value = value + +class RuntimeValue( object ): + """ + Wrapper to note a value that is not yet set, but will be required at + runtime. + """ + pass + def str_bool(in_str): """ diff --git a/lib/galaxy/web/controllers/workflow.py b/lib/galaxy/web/controllers/workflow.py index c4584960baa..bfbca2c506f 100644 --- a/lib/galaxy/web/controllers/workflow.py +++ b/lib/galaxy/web/controllers/workflow.py @@ -515,7 +515,7 @@ class WorkflowController( BaseController ): errors[step.id] = state.inputs["__errors__"] = step_errors # Connections by input name step.input_connections_by_name = dict( ( conn.input_name, conn ) for conn in step.input_connections ) - if not errors: + if 'run_workflow' in kwargs and not errors: # Run each step, connecting outputs to inputs outputs = odict() for step in workflow.steps: @@ -554,14 +554,13 @@ class WorkflowController( BaseController ): else: for step in workflow.steps: if step.type == 'tool' or step.type is None: - # Build a new tool state for the step + # Restore the tool state for the step tool = trans.app.toolbox.tools_by_id[ step.tool_id ] state = DefaultToolState() state.inputs = tool.params_from_strings( step.tool_inputs, trans.app ) # Store state with the step step.state = state - # This should never actually happen since we don't allow - # running workflows with errors (yet?) + # Error dict if step.tool_errors: errors[step.id] = step.tool_errors else: @@ -575,7 +574,8 @@ class WorkflowController( BaseController ): "workflow/run.mako", steps=workflow.steps, workflow=stored, - errors=errors ) + errors=errors, + incoming=kwargs ) @web.expose def configure_menu( self, trans, workflow_ids=None ): diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 07c3084f61f..374b41d0a78 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -1,7 +1,7 @@ from elementtree.ElementTree import Element from galaxy import web -from galaxy.tools.parameters import DataToolParameter, check_param +from galaxy.tools.parameters import DataToolParameter, RuntimeValue, check_param from galaxy.tools import DefaultToolState from galaxy.tools.parameters.grouping import Repeat, Conditional from galaxy.util.bunch import Bunch @@ -221,15 +221,34 @@ class ToolModule( object ): data_outputs.append( dict( name=name, extension=format ) ) return data_outputs def get_config_form( self ): - def as_html( param, value, trans, prefix ): - if type( param ) is DataToolParameter: - return "Data input '" + param.name + "' (" + ( " or ".join( param.extensions ) ) + ")" - else: - return param.get_html_field( trans, value ).get_html( prefix ) return self.trans.fill_template( "workflow/editor_tool_form.mako", - tool=self.tool, as_html=as_html, values=self.state.inputs, errors=( self.errors or {} ) ) + tool=self.tool, values=self.state.inputs, errors=( self.errors or {} ) ) def update_state( self, incoming ): - errors = self.tool.update_state( self.trans, self.tool.inputs, self.state.inputs, incoming ) + + print "STATE BEFORE UPDATE" + print self.state.inputs + + # Build a callback that handles setting an input to be required at + # runtime. We still process all other parameters the user might have + # set. + make_runtime_key = incoming.get( 'make_runtime', None ) + make_buildtime_key = incoming.get( 'make_buildtime', None ) + + def item_callback( trans, key, input, value, error, old_value, context ): + if key == make_buildtime_key: + return input.get_initial_value( trans, context ), None + elif isinstance( old_value, RuntimeValue ): + return old_value, None + elif key == make_runtime_key: + return RuntimeValue(), None + else: + return value, error + + # Update state using incoming values + errors = self.tool.update_state( self.trans, self.tool.inputs, self.state.inputs, incoming, item_callback=item_callback ) + + print "STATE UPDATED" + print self.state.inputs self.errors = errors or None diff --git a/static/scripts/galaxy.base.js b/static/scripts/galaxy.base.js index f343c31269a..3896b1ce5d2 100644 --- a/static/scripts/galaxy.base.js +++ b/static/scripts/galaxy.base.js @@ -65,9 +65,14 @@ function make_popupmenu( button_element, options ) { var click = function( e ) { var o = $(button_element).offset(); $("#popup-helper").mousedown( clean ).show(); - $( menu_element ).click( clean ).css( { top: -1000 } ).show().css( { + // Show off screen to get size right + $( menu_element ).click( clean ).css( { left: 0, top: -1000 } ).show(); + console.log( e.pageX, $(document).scrollLeft() + $(window).width(), $(menu_element).width() ); + var x = Math.min( e.pageX - 2, $(document).scrollLeft() + $(window).width() - $(menu_element).width() - 5 ); + + $( menu_element ).css( { top: e.pageY - 2, - left: e.pageX - 2 // + $(button_element).width() - $(menu_element).width() + left: x } ); return false; }; diff --git a/static/scripts/galaxy.panels.js b/static/scripts/galaxy.panels.js index e429c62043c..baaddddbbf3 100644 --- a/static/scripts/galaxy.panels.js +++ b/static/scripts/galaxy.panels.js @@ -194,30 +194,7 @@ function show_modal( title, body, buttons, extra_buttons ) { } }; -// Popup -- is this up to date? - -function make_popupmenu( button_element, options ) { - var menu_element = $( "
" ).appendTo( "body" ); - $.each( options, function( k, v ) { - $( "
" ).html( k ).click( v ).appendTo( menu_element ); - }); - var clean = function() { - $(menu_element).unbind().hide(); - $("#popup-helper").unbind().hide(); - }; - var click = function() { - var o = $(button_element).offset(); - $("#popup-helper").mousedown( clean ).show(); - $( menu_element ).click( clean ).css( { top: -1000 } ).show().css( { - top: o.top + $(button_element).height() + 9, - left: o.left + $(button_element).width() - $(menu_element).width() - } ); - }; - $( button_element ).click( click ); -}; - // Tab management - $(function() { $("span.tab").each( function() { diff --git a/templates/workflow/editor.mako b/templates/workflow/editor.mako index a91d1beeeaa..b8b27b88f27 100644 --- a/templates/workflow/editor.mako +++ b/templates/workflow/editor.mako @@ -21,6 +21,7 @@ ensure_dd_helper(); make_left_panel( $("#left"), $("#center"), $("#left-border" ) ); make_right_panel( $("#right"), $("#center"), $("#right-border" ) ); + ensure_popup_helper(); ## handle_minwidth_hint = rp.handle_minwidth_hint; @@ -39,6 +40,7 @@ +