mingshi-wang commented on pull request #20547:
URL: https://github.com/apache/airflow/pull/20547#issuecomment-1002760879


   > It's still not really backwards compatible (in the sense a having a New 
Sensor returnig PokeReturnValue run on airlfow pre 2.3).
   > 
   > If you return PokeReturnValue() in Airlfow 2.2. it will not work. The 
BaseSensorOperator from Airflow 2.2 will run this:
   > 
   > ```
   > while not self.poke():
   > ```
   > 
   > And it will always finish after first check no matter what "is_done" is. 
In this case self.poke() returns PokeReturnValue() class which is Truthy no 
matter what is set in `is_done`).
   > 
   > But It could be made backwards compatible - and I like this much more than 
my "return Truthy" approach.
   > 
   > Should be enough to do this:
   > 
   > ```
   > class PokeReturnValue:
   >     """
   >     Sensors can optionally return this an instance of this class in the 
poke method.
   >     If an xcom value is supplied when sensor is done, then the XCom value 
will be
   >     push through the operator return value.
   >     """
   > 
   >     def __init__(self, is_done, xcom_value=None):
   >         self.xcom_value = xcom_value
   >         self.is_done = is_done
   > 
   > 
   >     def __bool__(self):
   >        return self.is_done
   > ```
   > 
   > ~I think this is the BEST approach.~
   > 
   > UPDATE: Nope. It won't work, because in Airflow 2.2 there wil be no 
PokeReturnValue class.
   
   
   
   > It's still not really backwards compatible (in the sense a having a New 
Sensor returnig PokeReturnValue run on airlfow pre 2.3).
   > 
   > If you return PokeReturnValue() in Airlfow 2.2. it will not work. The 
BaseSensorOperator from Airflow 2.2 will run this:
   > 
   > ```
   > while not self.poke():
   > ```
   > 
   > And it will always finish after first check no matter what "is_done" is. 
In this case self.poke() returns PokeReturnValue() class which is Truthy no 
matter what is set in `is_done`).
   > 
   > But It could be made backwards compatible - and I like this much more than 
my "return Truthy" approach.
   > 
   > Should be enough to do this:
   > 
   > ```
   > class PokeReturnValue:
   >     """
   >     Sensors can optionally return this an instance of this class in the 
poke method.
   >     If an xcom value is supplied when sensor is done, then the XCom value 
will be
   >     push through the operator return value.
   >     """
   > 
   >     def __init__(self, is_done, xcom_value=None):
   >         self.xcom_value = xcom_value
   >         self.is_done = is_done
   > 
   > 
   >     def __bool__(self):
   >        return self.is_done
   > ```
   > 
   > ~I think this is the BEST approach.~
   > 
   > UPDATE: Nope. It won't work, because in Airflow 2.2 there wil be no 
PokeReturnValue class.
   
   A not-so-ideal way is to declare a dynamic return type:
   def poke(self, context: Context) -> Union[bool, 'PokeReturnValue']: 
      ...


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