hussein-awala commented on code in PR #35312:
URL: https://github.com/apache/airflow/pull/35312#discussion_r1378253597


##########
airflow/api/common/experimental/get_task_instance.py:
##########
@@ -42,5 +42,6 @@ def get_task_instance(dag_id: str, task_id: str, 
execution_date: datetime) -> Ta
     if not task_instance:
         error_message = f"Task {task_id} instance for date {execution_date} 
not found"
         raise TaskInstanceNotFound(error_message)
-
+    # API methods has access to the database.
+    assert isinstance(task_instance, TaskInstance)

Review Comment:
   assert statement is ignored in runtime; if you want to check this in 
runtime, you should switch to if...raise



##########
airflow/models/dagrun.py:
##########
@@ -466,7 +467,7 @@ def generate_run_id(run_type: DagRunType, execution_date: 
datetime) -> str:
     def fetch_task_instances(
         dag_id: str | None = None,
         run_id: str | None = None,
-        dag: DAG | None = None,
+        task_ids: list[str] | None = None,

Review Comment:
   This is a breaking change; we cannot remove an argument from a public 
method. Could you deprecate it instead, or even keep it and support both params?



##########
airflow/www/views.py:
##########
@@ -1543,6 +1543,7 @@ def rendered_k8s(self, *, session: Session = NEW_SESSION):
 
         pod_spec = None
         try:
+            assert isinstance(ti, TaskInstance)

Review Comment:
   same here for assert.



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to