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

pass through retry_policy for dbt assets #22143

Merged
merged 4 commits into from
May 30, 2024
Merged

pass through retry_policy for dbt assets #22143

merged 4 commits into from
May 30, 2024

Conversation

prha
Copy link
Member

@prha prha commented May 29, 2024

Summary & Motivation

There's no way to pass a retry policy to a dbt asset. I assumed this was mostly an oversight, and not an explicit decision.

Might need to sanity check that retries make sense in the dbt world.

In a lot of cases, users will probably want to run dbt retry (instead of op-level retry) to not lose the per-model granularity of failures, but there is a case to be made to still configure a coarse-grained op-level retry with the standard retry options (e.g. delay, jitter, etc)

How I Tested These Changes

BK

@prha prha requested review from sryza and rexledesma May 29, 2024 16:39
Copy link
Contributor

@rexledesma rexledesma left a comment

Choose a reason for hiding this comment

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

There was a previous PR that tried adding this and I pushed back: #18990

@prha prha closed this May 29, 2024
@prha prha reopened this May 30, 2024
@prha
Copy link
Member Author

prha commented May 30, 2024

We may still want to add this, so that users can specify delay/jitter in retrying a dbt job due to some resource constraint. This may be coarse-grained compared to dbt retry but that doesn't mean we shouldn't add it. Might have to do some specific docs help here to map out the different options.

@prha prha force-pushed the prha/dbt_retry_policy branch from 5eb9538 to 3cb978b Compare May 30, 2024 20:21
@prha prha merged commit b544128 into master May 30, 2024
1 check passed
@prha prha deleted the prha/dbt_retry_policy branch May 30, 2024 20:49
danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024
## Summary & Motivation
There's no way to pass a retry policy to a dbt asset. I assumed this was
mostly an oversight, and not an explicit decision.

Might need to sanity check that retries make sense in the dbt world.

In a lot of cases, users will probably want to run `dbt retry` (instead
of op-level retry) to not lose the per-model granularity of failures,
but there is a case to be made to still configure a coarse-grained
op-level retry with the standard retry options (e.g. delay, jitter, etc)

## How I Tested These Changes
BK

---------

Co-authored-by: Rex Ledesma <[email protected]>
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.

2 participants