Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Bugfix/nested non unit tasks #379

Merged
merged 3 commits into from
Jul 2, 2020
Merged

Bugfix/nested non unit tasks #379

merged 3 commits into from
Jul 2, 2020

Conversation

nh13
Copy link
Member

@nh13 nh13 commented Jul 2, 2020

@tfenne a very interesting edge case I hit in the real world

nh13 added 2 commits July 1, 2020 23:34
* Note: this commit shows the bug

Currently, a task that returns an empty task (`EmptyTask` via its
`getTasks` method would be stuck in the "ready" state.  It is not a
`UnitTasks` so isn't scheduled and executed, but it is also not an
`EmptyTask` so isn't considered "running".
Treat all non-`UnitTask` tasks that have no predecessors as "running",
and therefore eligible for completion.
@nh13 nh13 requested a review from tfenne July 2, 2020 06:38
@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2020

Codecov Report

Merging #379 into master will decrease coverage by 0.09%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #379      +/-   ##
==========================================
- Coverage   91.56%   91.47%   -0.10%     
==========================================
  Files          31       31              
  Lines        1186     1185       -1     
  Branches       80       80              
==========================================
- Hits         1086     1084       -2     
- Misses        100      101       +1     
Impacted Files Coverage Δ
.../main/scala/dagr/core/execsystem/TaskManager.scala 89.91% <94.11%> (-0.88%) ⬇️
.../main/scala/dagr/core/execsystem/TaskTracker.scala 94.11% <0.00%> (+1.17%) ⬆️

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 12c6036...ac2705c. Read the comment docs.

Copy link
Member

@tfenne tfenne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bunch of comments. Take what you will, leave the rest and then merge it!

@tfenne tfenne assigned nh13 and unassigned tfenne Jul 2, 2020
@nh13 nh13 merged commit c6c8fd9 into master Jul 2, 2020
@nh13 nh13 deleted the bugfix/nested-non-unit-tasks branch July 2, 2020 20:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants