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

Add more hooks to FlowRunner.initialize_run() #548

Merged
merged 6 commits into from
Jan 23, 2019
Merged

Conversation

jlowin
Copy link
Member

@jlowin jlowin commented Jan 21, 2019

Thanks for contributing to Prefect!

Please describe your work and make sure your PR:

  • updates CHANGELOG.md appropriately
  • is ready for review

Depends on #546

What does this PR change?

  • In preparation for more efficient ways of handling states provided by Prefect Cloud, this PR adds two new arguments to FlowRunner.initialize_run(): task_states and task_contexts. This gives the initialize_run method an opportunity to modify task-specific inputs. In this PR, it is only used to put some items in context (such as the task name and id)
  • Adds new "return types" for FlowRunner.initialize_run and TaskRunner.initialize_run -- this is simply a convenience to make it easier to statically type the library and access returned values
  • Adds tests for initialize_run() (it was lightly tested before)

Why is this PR important?

This PR stands alone by providing improved opportunity to interact with task states and contexts before a flow runs, but will mainly be used in a forthcoming PR that loads all task states from Cloud in a single call before the Flow runs. These hooks will make it possible to provide those states to the tasks.

@jlowin jlowin requested review from cicdw and joshmeek as code owners January 21, 2019 22:47
joshmeek
joshmeek previously approved these changes Jan 22, 2019
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

Some doc changes requested

Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

LGTM I approve

@jlowin jlowin merged commit ef7cd77 into master Jan 23, 2019
@jlowin jlowin deleted the flowrunner-hooks branch January 23, 2019 16:01
cicdw added a commit that referenced this pull request Nov 5, 2021
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.

3 participants