Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TASK LOOPING (in both Core and Cloud) #1356

Merged
merged 14 commits into from
Aug 13, 2019
Merged

TASK LOOPING (in both Core and Cloud) #1356

merged 14 commits into from
Aug 13, 2019

Conversation

cicdw
Copy link
Member

@cicdw cicdw commented Aug 13, 2019

Thanks for contributing to Prefect!

Please describe your work and make sure your PR:

  • adds new tests (if appropriate)
  • updates CHANGELOG.md (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

Note that your PR will not be reviewed unless all three boxes are checked.

What does this PR change?

This PR introduces "Task Looping" (first described in PIN 11). The implementation here will actually work in Cloud almost immediately (once the Cloud serializers are updated).

In order to make Looping work off-the-shelf, this PR:

  • introduces a new Looped state which carries the loop_count
  • introduces a new LOOP signal for triggering a loop
  • places the appropriate data in context (loop count + loop result)
  • re-uses our cached_inputs mechanism for conveying relevant data to retries
  • allows for loops to begin at arbitrary points by setting the appropriate task_loop_count in context

Why is this PR important?

This PR furthers the ability for Flows to dynamically spawn tasks. Mapping is to for loops as Looping is to while loops.

This of course will need extensive documentation to fully expose.

@codecov
Copy link

codecov bot commented Aug 13, 2019

Codecov Report

Merging #1356 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@ -467,6 +476,32 @@ class Finished(State):
color = "#003ccb"


class Looped(Finished):
Copy link
Member

Choose a reason for hiding this comment

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

Makes me wonder if both Looped and Mapped should become Looping and Mapping since they imply that more work is going to be done

Copy link
Member Author

@cicdw cicdw Aug 13, 2019

Choose a reason for hiding this comment

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

Yea I thought about this; I'm happy to change it but here's my reasoning for Looped: when a Task enters a Looped state it lets us know that a single iteration of the Loop was complete. The full "looping" mechanism actually involves Looped -> Running -> Looped -> Running.

This is in contrast to a Running state - when a Task is in a Running state, it actually means there is work currently being done.

Copy link
Member

Choose a reason for hiding this comment

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

I'm in

@cicdw cicdw merged commit 3d2a26f into master Aug 13, 2019
@cicdw cicdw deleted the looping-take-2 branch August 13, 2019 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants