amoghrajesh commented on code in PR #53389:
URL: https://github.com/apache/airflow/pull/53389#discussion_r2266561248
##########
airflow-core/src/airflow/example_dags/example_latest_only_with_trigger.py:
##########
@@ -29,7 +29,7 @@
from airflow.providers.standard.operators.empty import EmptyOperator
from airflow.providers.standard.operators.latest_only import LatestOnlyOperator
from airflow.sdk import DAG
-from airflow.utils.trigger_rule import TriggerRule
+from airflow.sdk.api.datamodels._generated import TriggerRule
Review Comment:
This will not be backward compatible. If someone copy pastes this dag in
their v3.0.4 Airflow, it will start failing. We should have the imports in
example dags be backward compatible.
##########
task-sdk/src/airflow/sdk/api/datamodels/_generated.py:
##########
@@ -472,6 +472,22 @@ class TaskInstanceState(str, Enum):
DEFERRED = "deferred"
+class TriggerRule(str, Enum):
Review Comment:
Can we add a lazy import for this? I do not like the import pattern of:
`from airflow.sdk.api.datamodels._generated import TriggerRule`
It could be `from airflow.sdk import TriggerRule`
##########
airflow-core/tests/unit/models/test_dag.py:
##########
@@ -1350,7 +1350,6 @@ def test_create_dagrun_job_id_is_set(self):
def test_dag_add_task_checks_trigger_rule(self):
# A non fail stop dag should allow any trigger rule
from airflow.exceptions import FailFastDagInvalidTriggerRule
Review Comment:
This will probably fail now? The `add_task` in dag will now raise
`FailFastDagInvalidTriggerRule` from sdk class but we are trying to assert from
`FailFastDagInvalidTriggerRule` from `airflow.exceptions`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
