[GitHub] [airflow] yuqian90 commented on a change in pull request #8702: Add context to execution_date_fn in ExternalTaskSensor

[GitBox <gi...@apache.org>]

yuqian90 commented on a change in pull request #8702:
URL: https://github.com/apache/airflow/pull/8702#discussion_r497897460



##########
File path: airflow/sensors/external_task_sensor.py
##########
@@ -164,6 +164,26 @@ def poke(self, context, session=None):
         session.commit()
         return count == len(dttm_filter)
 
+    def _handle_execution_date_fn(self, context):
+        """
+        This function is to handle backwards compatibility with how this operator was
+        previously where it only passes the execution date, but also allow for the newer
+        implementation to pass all context through as well, to allow for more sophisticated
+        returns of dates to return.
+        Namely, this function check the number of arguments in the execution_date_fn
+        signature and if its 1, treat the legacy way, if it's 2, pass the context as
+        the 2nd argument, and if its more, throw an exception.
+        """
+        num_fxn_params = self.execution_date_fn.__code__.co_argcount
+        if num_fxn_params == 1:
+            return self.execution_date_fn(context['execution_date'])
+        elif num_fxn_params == 2:
+            return self.execution_date_fn(context['execution_date'], context)
+        else:
+            raise AirflowException(

Review comment:
       This change stops people from doing loop variable capture. There are more friendly ways to allow compatibility, such as using `inspect.signature` to figure out the actual argument names.
   
   ```python
       for day in range(1, 7):
           sensor = ExternalTaskSensor(
               task_id="sensor_{}".format(day),
               external_dag_id=external_dag_id,
               external_task_id=external_task_id,
               execution_date_fn=lambda execution_date, local_day=day: ...,
           )
   ```




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

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