vincbeck commented on code in PR #38992:
URL: https://github.com/apache/airflow/pull/38992#discussion_r1605048275


##########
airflow/datasets/manager.py:
##########
@@ -71,13 +76,14 @@ def register_dataset_change(
         For local datasets, look them up, record the dataset event, queue 
dagruns, and broadcast
         the dataset event
         """
+        # todo: add test so that all usages of internal_api_call are added to 
rpc endpoint

Review Comment:
   Not sure if you want to do that later or you forgot this todo, just calling 
it out here :)



##########
airflow/models/taskinstance.py:
##########
@@ -1265,6 +1558,130 @@ def _update_rtif(ti, rendered_fields, session: Session 
| None = None):
     RenderedTaskInstanceFields.delete_old_records(ti.task_id, ti.dag_id, 
session=session)
 
 
+def _coalesce_to_orm_ti(*, ti, session: Session):

Review Comment:
   ```suggestion
   def _coalesce_to_orm_ti(*, ti: TaskInstance | TaskInstancePydantic, session: 
Session):
   ```



##########
airflow/models/taskinstance.py:
##########
@@ -2709,43 +3004,12 @@ def _execute_task(self, context: Context, task_orig: 
Operator):
         return _execute_task(self, context, task_orig)
 
     @provide_session
-    def defer_task(self, session: Session, defer: TaskDeferred) -> None:
+    def defer_task(self, exception: TaskDeferred, session: Session) -> None:

Review Comment:
   Out of curiosity why renaming `defer` to `exception`?



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