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

Update deployment flow run retry settings with runtime values #6489

Merged
merged 2 commits into from
Aug 22, 2022

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Aug 19, 2022

Summary

Retries are configured on ad-hoc flow runs during flow run creation. When flow runs are created for deployments, settings are copied from the deployment to the flow run. However, deployments do not have any concept of flow run orchestration policies yet. Instead of introducing retries to deployments now, we will update the flow run with any discovered orchestration settings at runtime. In the future, the deployment may override these discovered settings by setting values on flow run creation. With the changes in #6488 we should not need to change this code for that behavior to take effect :)

Requires #6486 and #6488
Closes #6485

Checklist

This pull request is:

  • A documentation / typographical error fix
    • No tests or issue needed
  • A short code fix
    • Please reference the related issue by including "closes <link to issue>" in this Pull Request's summary section.
      • If no issue exists, please create a bug report issue
    • Please include tests. One-line fixes without tests will not be accepted unless it's related to the documentation only.
  • A new feature implementation
    • Please reference the related issue by including "closes <link to issue>" in this Pull Request's summary section.
      • If no issue exists, please create a feature enhancement issue
    • Please include tests
    • Please make sure that your QA steps are both thorough and easy to reproduce by somebody with limited knowledge of the feature that you are submitting

Happy engineering!

Base automatically changed from update-policy to main August 19, 2022 19:21
@zanieb zanieb force-pushed the fix-deployment-flow-retries branch from 21d006a to 230659f Compare August 19, 2022 19:22
@zanieb zanieb changed the base branch from main to policy-optional August 19, 2022 19:23
@zanieb zanieb force-pushed the fix-deployment-flow-retries branch from 230659f to d5e6543 Compare August 19, 2022 19:24
@zanieb zanieb marked this pull request as ready for review August 19, 2022 19:24
@zanieb zanieb requested a review from cicdw as a code owner August 19, 2022 19:24
@zanieb zanieb added the bug Something isn't working label Aug 19, 2022
Copy link
Contributor

@anna-geller anna-geller left a comment

Choose a reason for hiding this comment

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

Excellent! works well

image

Base automatically changed from policy-optional to main August 19, 2022 21:00
Retries are configured on ad-hoc flow runs during flow run creation. When flow runs are created for deployments, settings are copied from the deployment to the flow run. However, deployments do not have any concept of flow run orchestration policies yet. Instead of introducing retries to deployments now, we will update the flow run with any discovered orchestration settings at runtime. In the future, the deployment will have the option of overriding these discovered settings.
@zanieb zanieb force-pushed the fix-deployment-flow-retries branch from d5e6543 to ebf383d Compare August 19, 2022 22:58
@zanieb zanieb merged commit 39b3905 into main Aug 22, 2022
@zanieb zanieb deleted the fix-deployment-flow-retries branch August 22, 2022 14:41
darrida pushed a commit to darrida-forked/prefect that referenced this pull request Aug 25, 2022
…tHQ#6489)

* Update deployment flow run retry settings with runtime values

Retries are configured on ad-hoc flow runs during flow run creation. When flow runs are created for deployments, settings are copied from the deployment to the flow run. However, deployments do not have any concept of flow run orchestration policies yet. Instead of introducing retries to deployments now, we will update the flow run with any discovered orchestration settings at runtime. In the future, the deployment will have the option of overriding these discovered settings.

* Add test
@zanieb zanieb added fix A fix for a bug in an existing feature and removed bug Something isn't working labels Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug in an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flow-level retries don't work with deployed flow runs
3 participants