Re: [PR] Add executor field to the DB and parameter to the operators [airflow]

Thu, 14 Mar 2024 11:24:14 -0700 


o-nikolas commented on code in PR #38054:
URL: https://github.com/apache/airflow/pull/38054#discussion_r1525325297


##########
airflow/models/taskinstance.py:
##########
@@ -1252,6 +1254,8 @@ class TaskInstance(Base, LoggingMixin):
     queued_dttm = Column(UtcDateTime)
     queued_by_job_id = Column(Integer)
     pid = Column(Integer)
+    # TODO: Should this be nullable?
+    executor = Column(String(1000))

Review Comment:
   This is actually a nuanced bit that's easy to get confused by. The default 
must be NULL/empty to maintain the behaviour we have today in the base case. I 
covered this in the AIP and I think that is the best language I've come up 
with, so I've attached that below. As well as comment threads from Jens and 
Jarek who +1 the idea during review:
   
   Explanation from AIP:
   ![Screenshot from 2024-03-14 
11-21-29](https://github.com/apache/airflow/assets/65743084/79de991d-b060-45fd-aeb4-055985de3485)
   
   Comment thread:
   ![Screenshot from 2024-03-14 
11-21-42](https://github.com/apache/airflow/assets/65743084/410cbbf1-26cc-41c3-9ac4-b9458354078b)
   ![Screenshot from 2024-03-14 
11-21-51](https://github.com/apache/airflow/assets/65743084/23984b47-84bd-4030-abe0-e6a0df8d5bb8)
   ![Screenshot from 2024-03-14 
11-22-00](https://github.com/apache/airflow/assets/65743084/89f12dba-98d2-40ad-9ff4-6cc41cd1a311)
   



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