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

Add support for empty tasks #360

Merged
merged 1 commit into from
Aug 22, 2019
Merged

Add support for empty tasks #360

merged 1 commit into from
Aug 22, 2019

Conversation

nh13
Copy link
Member

@nh13 nh13 commented Aug 22, 2019

  1. Tasks.getTasks can now return an empty list of tasks to run (ex. None, Seq.empty) and the execution system will set the parent task to completed.
  2. Added a Tasks$.empty method to create a small task whose getTasks method returns None.

@nh13 nh13 requested a review from tfenne August 22, 2019 04:51
@nh13 nh13 mentioned this pull request Aug 22, 2019
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.

LGTM.

core/src/main/scala/dagr/core/tasksystem/Task.scala Outdated Show resolved Hide resolved
@@ -128,10 +128,19 @@ class TaskManagerTest extends UnitSpec with OptionValues with LazyLogging with B
tryTaskNTimes(taskManager = taskManager, task = task, numTimes = 1, taskIsDoneFinally = true, failedAreCompletedFinally = true)
}

it should "run an empty task" in {
Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to include a test that has a small pipeline of realTask ==> Task.empty ==> anotherRealTask.

@tfenne
Copy link
Member

tfenne commented Aug 22, 2019

One other thought would be whether it would make sense to add something like def EmptyTask: Task = Task.empty to Pipeline?

@nh13 nh13 assigned nh13 and unassigned tfenne Aug 22, 2019
@codecov-io
Copy link

Codecov Report

Merging #360 into master will increase coverage by 0.19%.
The diff coverage is 93.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
+ Coverage   91.84%   92.03%   +0.19%     
==========================================
  Files          31       31              
  Lines        1140     1155      +15     
  Branches       71       66       -5     
==========================================
+ Hits         1047     1063      +16     
+ Misses         93       92       -1
Impacted Files Coverage Δ
...src/main/scala/dagr/core/tasksystem/Pipeline.scala 85% <0%> (-4.48%) ⬇️
...ore/src/main/scala/dagr/core/tasksystem/Task.scala 96.96% <100%> (+0.14%) ⬆️
.../main/scala/dagr/core/execsystem/TaskManager.scala 92.85% <96.55%> (+1.3%) ⬆️

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 0c5364a...86e749b. Read the comment docs.

@nh13 nh13 merged commit 2b5adf6 into master Aug 22, 2019
@nh13 nh13 deleted the nh_gettasks_none branch August 22, 2019 18:33
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