Lee-W commented on code in PR #46633:
URL: https://github.com/apache/airflow/pull/46633#discussion_r1955460470


##########
providers/standard/src/airflow/providers/standard/triggers/external_task.py:
##########
@@ -143,11 +141,11 @@ def _get_count(self, states: typing.Iterable[str] | None) 
-> int:
 
 class DagStateTrigger(BaseTrigger):
     """
-    Waits asynchronously for a DAG to complete for a specific logical date.
+    Waits asynchronously for a DAG to complete for a specific run_id.

Review Comment:
   ```suggestion
       Waits asynchronously for a dag to complete for a specific run_id.
   ```
   according to the discussion in 
https://lists.apache.org/thread/lktrzqkzrpvc1cyctxz7zxfmc0fwtq2j, I think we 
should use dag instead



##########
providers/standard/src/airflow/providers/standard/triggers/external_task.py:
##########
@@ -143,11 +141,11 @@ def _get_count(self, states: typing.Iterable[str] | None) 
-> int:
 
 class DagStateTrigger(BaseTrigger):
     """
-    Waits asynchronously for a DAG to complete for a specific logical date.
+    Waits asynchronously for a DAG to complete for a specific run_id.
 
     :param dag_id: The dag_id that contains the task you want to wait for
     :param states: allowed states, default is ``['success']``
-    :param logical_dates: The logical date at which DAG run.
+    :param run_ids: The run_id of DAG run.

Review Comment:
   ```suggestion
       :param run_ids: The run_id of dag run.
   ```



##########
providers/standard/src/airflow/providers/standard/triggers/external_task.py:
##########
@@ -55,7 +55,7 @@ class WorkflowTrigger(BaseTrigger):
     def __init__(
         self,
         external_dag_id: str,
-        logical_dates: list[datetime] | None = None,
+        run_ids: list[datetime] | None,

Review Comment:
   ```suggestion
           run_ids: list[str] | None,
   ```
   
    I think run_id is str?



##########
tests/operators/test_trigger_dagrun.py:
##########
@@ -41,6 +40,7 @@
 pytestmark = pytest.mark.db_test
 
 DEFAULT_DATE = datetime(2019, 1, 1, tzinfo=timezone.utc)
+DEFAULT_RUN_ID = f"{DagRunType.MANUAL}__{DEFAULT_DATE.isoformat()}"

Review Comment:
   Should we add a case that contains random string (as the logical_date could 
be none)



##########
providers/standard/src/airflow/providers/standard/triggers/external_task.py:
##########
@@ -156,23 +154,21 @@ def __init__(
         self,
         dag_id: str,
         states: list[DagRunState],
-        logical_dates: list[datetime] | None = None,
+        run_ids: list[datetime] | None,

Review Comment:
   ```suggestion
           run_ids: list[str] | None,
   ```



-- 
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]

Reply via email to