Detail bug report from Michael Crusoe here : https://trello.com/c/0mdGCx4P.
This also fixes a test case added in 289e48b which was both attempting to assert something wrong and was incorrectly implemented. Augmenting the workflow editor test suite with some actually valid test cases that assert the correct behaviors.
For a longer explaination - workflow input terminals have two related concepts 'canAccept' and 'attachable'. An output terminal is 'attachable' if in the abstract it could be attached to the input regardless of whether the input is already filled or not. 'canAccept' is more stateful in that an output terminal is 'canAccept'able if the input terminal is not filled ('_inputFilled') and it is 'attachable'.
So - the problem was 'attachable' was not correctly defined for a multiple input data parameters. 'attachable' was asserting that any connected input terminal could not be 'attachable' by an collection output terminal - so on these asynchronous node state changes collections attached to multiple input data parameters were being wiped out. The more percise/correct distinction is that if a multiple input data parameter has single inputs connected to it - it cannot also have a collection connected to it (yet anyway). This fixes the mentioned bug.
The problematic test case was conflating 'attachable' and 'canAccept'able - I have fixed the test case to verify the correct 'attachable' logic and added newer, higher level test cases to test the canAccept logic and the actual behavior the end user would observe (of the connector being destroy).
Actually do some verification of imported history, eliminiate use of deprecated mixin, timeout operations that 'wait', break up big function and name test better.
In particular break out mixins defining context-based utilities for interacting with app, an user, and a history. In downstream work on workflow scheduling this proves nessecary and sufficient to create an context for 'execute'-ing tools (create jobs) outside of web threads in response to workflow requests.
Unit tests for some of this.
Add utility to RuleHelper to simplify bursting reasoning slightly, setup a stock rule for bursting between two static job destinations - mostly for demonstration purprose but it might be useful in some settings.
... if only in a limitted sort of way. Provide ability to hash jobs by history, workflow, user, etc... and choose among various destinations semi-randomly based on that to distribute work across destinations based on these factors.
Add a stock rule as in the form of a new dynamic destination type ("choose_one") that demonstrate using this to quickly proxy to some fixed static destinations. Add examples to job_conf.xml.sample_advanced.
Generate a workflow invocation UUID that is stored with each job in the workflow and allow dynamic job destinations to consume these. Should allow for grouping jobs from the same workflow together during resource allocation (a sort of first attempt at dealing with data locality in workflows).
In other words, send extra job destination parameters to dynamic rule functions as arguments (in addition to those dynamically populated by Galaxy itself). This enables greater parameterization of rule functions and should lead to cleaner separation of logic and data (i.e. sites can program rules that restrict access to users, but which users can be populated at a higher level in `job_conf.xml`).
For example the following dynamic job rule:
def cluster1(app, memory="4096", cores="1", hours="48"):
native_spec = "--time=%s:00:00 --nodes=1 --ntasks=%s --mem=%s" % ( hours, cores, memory )
return JobDestination( "cluster1", params=dict( native_specification=native_spec ) )
Could then be called with various parameters in job_conf.xml as follows:
<destination id="short_job" type="dyanmic">
<param id="function">cluster1</param>
<param id="hours">1</param>
</destination>
<destination id="big_job" type="dynamic">
<param id="function">cluster1</param>
<param id="cores">8</param>
<param id="memory">32768</param>
</destination>
If multiple destinations map to the same underlying resource (a very typical case), this could allow rule developer to reason about cluster as a whole - though perhaps clunkily - by supplying all destination ids mapping to that cluster.
A bunch of unit tests for galaxy.jobs.mapper and galaxy.jobs.rule_helper testing various aspects of static and dynamic job mapping and the utility class (RuleHelper) that can be supplied to dynamic job rules.
... this will allow linking to actual steps (referential integrity) if/when we store workflow requests in the database and failing faster.
Also more unit tests.
Convert workflow API to use new-style API decorator throughout.
Improved workflow exception handling - now more explicit status code setting and returning of raw error strings in the API. Most obvious exceptional paths out of the API are now coming through MessageExceptions of things even more specific. Add API tests for many of these exceptional conditions.
Use more helper methods to reduce method length, duplication.
Previous route was "POST /api/workflows/upload", this still works but should be condisdered deprecated in favor
of POSTing to /api/workflows with 'workflow' in the payload.
... so internals of tool XML description are only utilized in the galaxy.tools module (and submodules). Add some unit tests for this new method for finding externally referenced files.
The old 'ds_map' parameter remains in place for backward compatibility and with the same behavior. A new parameter 'inputs' can now be specified instead however, and its keys corresponding to the steps 'order_index' instead of the raw unencoded database ids 'ds_map' uses. This variant has the nice property that this map can be constructed without prior knowledge of how Galaxy will assign ids during import - hence it is easier to use and more portable.
Additionally, 'inputs' is more flexiable and can revert to the old behavior by specifying a new parameter 'inputs_by' as 'step_id'. 'inputs_by' can also be 'name' - this is even more human friendly because it will assign the ids based on data input names (this is what I intend to use mostly, but I have not made it the default because not all workflows will have data inputs with distinct names).
Finally, the new name 'inputs' will be more appropriate once data collection inputs can be explicitly mapped via the API.
John Chilton