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

Orchestrators refactoring and fork handling other orchestrators #70

Closed tiagofilipe12 closed 6 years ago

tiagofilipe12 commented 6 years ago

This pull request includes a patch on uid for spamming tasks with junction as discussed in #69. Though this is still incomplete the remaining code of the PR can be quite complex and so we should merge it with dev branch before fixing uids for good. In fact, fork helped a lot to understand how we could handle duplication of tasks inside a pipeline, though merkle tree like handling of uids might be better.

This PR uniforms the resulting keys from each orchestrator .info, e.g.:

joinInvocator.info = {
  tasks: tasks.map(task => task),  //returns an array of tasks
  type: 'join'  // used to check the type of orchestrator
}

Now, each orchestrator stores an array of tasks that are inside them, e.g., join(task1, task2) results in [task1, task2] in which each entry is the taskInvocator function or the orchestrator (join(task1, fork(task2, task3) results in [task1, fork(task2,task3)] where orchestrator is handled as tasks.

Also and more importantly fork suffered a major refactoring (to match the other two orchestrators 'shape' and to be able to handle properly junction and fork inside fork. To do so, fork inside fork needs to store outer fork downstream tasks, inside outer fork it has to store upstream tasks and downstream tasks for this inner fork and the tasks inside this fork as well. Similar procedures was adopted to handle junction. However, they differ in the way lineage is stored. While fork forces duplication of tasks by adding a new uid for each task inside inner fork and adds a new lineage for each task inside fork (see code here), junction just adds downstream and upstream tasks inside the fork, as well as the newResttasks (tasks after the fork) to the junction itself.

Also, in this PR a bunch of test cases were added to ./examples/pipelines/tests in order to test these changes in the orchestrators as well as the 'orchestration' between them.

I am still working to simplify the code and document it in GSoC17 repo, so don't merge it yet. Just check if you want to change anything or if you can test it (with my examples/pipelines/tests or other that you find suitable or challenging enough :smile:. I also want to stress test it a little more.

tiagofilipe12 commented 6 years ago

Progress on this PR is being documented here. Meanwhile, merkle tree like uid generation was added to task creation in order to avoid duplication of tasks within the same pipeline (but still needs testing before merging).

tiagofilipe12 commented 6 years ago

Just refactored orchestrators function to have .uid as tasks. And re-started refactor on fork. Currently, fork just doesn't handle other forks, everything else seems to be properly working in current state.

codecov-io commented 6 years ago

Codecov Report

Merging #70 into dev will decrease coverage by 0.29%. The diff coverage is 73.17%.

Impacted file tree graph

@@           Coverage Diff            @@
##              dev     #70     +/-   ##
========================================
- Coverage   80.49%   80.2%   -0.3%     
========================================
  Files          36      37      +1     
  Lines         841     884     +43     
  Branches      101     104      +3     
========================================
+ Hits          677     709     +32     
- Misses        164     175     +11
Impacted Files Coverage Δ
lib/constants/default-task-state.js 100% <ø> (ø) :arrow_up:
lib/reducers/collection.js 85.55% <0%> (ø) :arrow_up:
lib/orchestrators/orchestrator-uid-generator.js 100% <100%> (ø)
lib/lifecycle/generate-uid.js 100% <100%> (ø) :arrow_up:
lib/watermill.js 90.47% <100%> (ø) :arrow_up:
lib/orchestrators/join.js 96.66% <100%> (+1.21%) :arrow_up:
lib/orchestrators/fork.js 37.93% <28%> (-12.07%) :arrow_down:
lib/orchestrators/junction.js 89.28% <75%> (+0.39%) :arrow_up:
lib/task.js 96.55% <91.66%> (-3.45%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 20d56f3...d16531a. Read the comment docs.

codecov-io commented 6 years ago

Codecov Report

Merging #70 into dev will increase coverage by 1.38%. The diff coverage is 95%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #70      +/-   ##
==========================================
+ Coverage   80.49%   81.88%   +1.38%     
==========================================
  Files          36       37       +1     
  Lines         841      861      +20     
  Branches      101      102       +1     
==========================================
+ Hits          677      705      +28     
+ Misses        164      156       -8
Impacted Files Coverage Δ
lib/constants/default-task-state.js 100% <ø> (ø) :arrow_up:
lib/reducers/collection.js 85.55% <0%> (ø) :arrow_up:
lib/task.js 100% <100%> (ø) :arrow_up:
lib/orchestrators/orchestrator-uid-generator.js 100% <100%> (ø)
lib/lifecycle/generate-uid.js 100% <100%> (ø) :arrow_up:
lib/orchestrators/join.js 96.49% <100%> (+1.03%) :arrow_up:
lib/watermill.js 90.47% <100%> (ø) :arrow_up:
lib/orchestrators/fork.js 100% <100%> (+50%) :arrow_up:
lib/orchestrators/junction.js 89.28% <75%> (+0.39%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 20d56f3...8cdd6f5. Read the comment docs.