-
Notifications
You must be signed in to change notification settings - Fork 199
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
Implement no_retries_on_test_failure
on DbtTestLocalOperator
#1563
base: main
Are you sure you want to change the base?
Implement no_retries_on_test_failure
on DbtTestLocalOperator
#1563
Conversation
✅ Deploy Preview for sunny-pastelito-5ecb04 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
no_retries_on_test_failure
on DbtTestLocalOperator
440201c
to
3307086
Compare
3307086
to
47ed26c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @okayhooni, thanks a lot for the report and your contribution!
Apologies for the delayed review—I was out last week and have been catching up this week.
After discussing this internally with @tatiana, we feel it would be better to unify this configuration with our existing interface for similar scenarios. For reference, in PR #1492, we added support for overriding the profile per dbt node. Similarly, we think it would be more consistent to allow this configuration to be overridden at the test level for specific nodes and at the dbt_project.yml
level for global application.
For example, similar to this configuration, we could support something like:
tests:
jaffle_shop:
+meta:
cosmos:
retries: 0
Another point to consider is that test failures may also occur due to connectivity issues, such as being unable to connect to the datasource. In such cases, it might make sense to retry the task. If we can determine the actual reason for the failure—whether it was due to a failed dbt check or a connectivity issue—then we could apply this configuration accordingly (perhaps by analyzing the logs).
Additionally, we might not want to disable retries by default, as it maybe considered breaking change for existing versions. Right now, tasks may retry for various reasons beyond dbt test failures, such as network interruptions.
Looking forward to your thoughts on this!
Description
Added
no_retries_on_test_failure
parameter toDbtTestLocalOperator
with a default value ofTrue
.This parameter raises
AirflowFailException
, ensuring that failed DBT tests are not retried unnecessarily, even if a common retries value of 1 or more is set inDbtDag
/DbtTaskGroup
.Unlike the
on_warning_callback
parameter, it does not seem necessary to propagateno_retries_on_test_failure
through theconverter
andairflow/graph
modules. Instead, similar to thecallback
parameter inAbstractDbtLocalBase
, it can be passed viaoperator_args
when needed.Related Issue(s)
closes #1537
Breaking Change?
Nope.
Checklist