bionode / bionode-watermill

💧Bionode-Watermill: A (Not Yet Streaming) Workflow Engine
https://bionode.gitbooks.io/bionode-watermill/content/
MIT License
37 stars 11 forks source link

Refactor task definition #75

Open tiagofilipe12 opened 7 years ago

tiagofilipe12 commented 7 years ago

Current implementation on task definition by the user is somehow javascript dependent. Example:

const gunzipIt = task({
    input: '*_genomic.fna.gz',
    output: '*.fa',
    params: { output: 'uncompressed.fa' }
  }, ({ params, input}) => `gunzip -c ${input} > ${params.output}`
)

It would be nice to abstract the user from this object, function task shape, to something more user-friendly. For example:

const gunzipIt = { task:
  { input:  '*_genomic.fna.gz',
    output: 'uncompressed.fa',
    function: '`gunzip -c ${input} > ${params.output}`'
  }
}

Notice that I removed params.output, because that can be tricky for the user to understand, and instead this check of the output pattern could be done by the api. This could be integrated with #74 .

thejmazz commented 7 years ago

Why

'`gunzip -c ${input} > ${params.output}`'

instead of

`gunzip -c ${input} > ${params.output}`

?

Except then input and params will be undefined: I think we will always still need something with

func: function(resolvedProps) {
  return `gunzip -c ${resolvedProps.input} > ${params.output}`
}

and then, the change is basically just moving the operationCreator from the second param to a property of the props object. Which I am OK with as a change -

const gunzipIt = {
  input:  '*_genomic.fna.gz',
  output: 'uncompressed.fa',
  operation: ({ params, input}) => `gunzip -c ${input} > ${params.output}`
}

(maybe don't need to nest under task? - except for orchestrators?)

One benefit of having the task function is that you still then get an invocableTask which can be used to manually orchestrate tasks or anything.

Either way - I think this is less priority than #74 and probably mostly a style change, unless #74 brings up changes that require changing it here as well.

It feels like to me this simply just changes (propsObject, operationCreator) into (propsObjectWithOperationCreator) since the task function is still needed to parse the definition into something that runs a task lifecycle - invocableTask

tiagofilipe12 commented 7 years ago

I said:

'`gunzip -c ${input} > ${params.output}`'

because internally maybe it can be converted into this:

func: function(resolvedProps) {
  return `gunzip -c ${resolvedProps.input} > ${params.output}`
}

but your suggestion is also nice, where you replace (propsObject, operationCreator) by (propsObjectWithOperationCreator).

And yes, this is just styling to make it easier for users outside the scope of JS to use the tool.

tiagofilipe12 commented 7 years ago

https://blog.jcoglan.com/2011/03/11/promises-are-the-monad-of-asynchronous-programming/