Fix test_roundtrip_provider_example_dags #49647
Conversation
|
created issue here #49648. |
|
|
||
| if field == "max_consecutive_failed_dag_runs": | ||
| # TaskSDK sets -1 default to max_consecutive_failed_dag_runs | ||
| assert actual in [expected, 0], f"{dag.dag_id}.{field} does not match" |
There was a problem hiding this comment.
@ashb @amoghrajesh @kaxil -> just checking if that was intentional change to change it it to -1 ? max_consecutive_runs is experimental so should not matter, just checking
There was a problem hiding this comment.
Merrging to unblock main without waiting
There was a problem hiding this comment.
I really do not recall why we did this, maybe there was no good reason to do it at all.
There was a problem hiding this comment.
Maybe it was -1 in the serialization side and set to default from config elsewhere? Or perhaps it was when we hadn't yet worked out what we would do for config in the task-sdk dist yet.
There was a problem hiding this comment.
Yes the default value zero being set here https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/dag.py#L438. so should we align all with zero?
5 participants
Alright bit of context here:
The test validates a minmum of 7 dags serialised or not. until it was working fine, recently edge3 example_dag converted to TasSDK dag. Then the tests failing to validate
max_consecutive_failed_dag_runsas TaskSDk sets to default to -1 and serialised dagbag one have default to zero.https://github.com/apache/airflow/blob/main/task-sdk/src/airflow/sdk/definitions/dag.py#L403
https://github.com/apache/airflow/actions/runs/14621219429/job/41021936045?pr=49631#step:6:1909
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.