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

Fix local retries in Cloud to always run in-process #1348

Merged
merged 2 commits into from
Aug 12, 2019
Merged

Conversation

cicdw
Copy link
Member

@cicdw cicdw commented Aug 11, 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 makes an additional call to get_task_run_info whenever tasks have to retry locally. Previously, prefect context was reset to its original state, meaning the task run version that was present in context was stale on the next run, preventing the retry from actually occurring in process (instead it occurred in a new run). Note that local retries were properly working for mapped tasks because of the additional call to get_task_run_info that mapped tasks always perform prior to a run.

Why is this PR important?

It is much more efficient to retry in the same process for short duration retries.

assert (
client.set_task_run_state.call_count == 5
) # Pending -> Running -> Failed -> Retrying -> Running -> Success
versions = [call[1]["version"] for call in client.set_task_run_state.call_args_list]
assert versions == [1, 2, 3, 4, 5]
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously this test failed because versions = [1, 2, 3, 1, 2]

@codecov
Copy link

codecov bot commented Aug 11, 2019

Codecov Report

Merging #1348 into master will increase coverage by <.01%.
The diff coverage is 100%.

@cicdw cicdw merged commit 714a861 into master Aug 12, 2019
@cicdw cicdw deleted the patch-local-retries branch August 12, 2019 02:56
@zdhughes zdhughes restored the patch-local-retries branch August 23, 2019 18:37
@cicdw cicdw deleted the patch-local-retries branch November 3, 2019 01:17
abrookins pushed a commit that referenced this pull request Mar 15, 2022
…sign

Clean up route guards. Use panel guard from orion-design
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