SamWheating opened a new pull request, #70684: URL: https://github.com/apache/airflow/pull/70684
Maybe more of a discussion topic here - Many sensors will define their own `execute()` method, which then conditionally calls out to the deferable / non-deferable path. However, we're pretty inconsistent about whether or not that value should be returned or not. Some sensors will return the value of `execute()` - some sensors will return the result: https://github.com/apache/airflow/blob/cd3a7e1a5f6bbe9f6536ab593b06a5728deb0271/providers/http/src/airflow/providers/http/sensors/http.py#L160-L162 https://github.com/apache/airflow/blob/4b18ed29a42f8489bb87c97bcb41078ca76c867d/providers/google/src/airflow/providers/google/cloud/sensors/gcs.py#L499-L504 but most sensors will not: https://github.com/apache/airflow/blob/8dd76f1624bc28fdef8630684a06914891526578/providers/standard/src/airflow/providers/standard/sensors/filesystem.py#L120-L122 This makes for a really confusing experience when trying to subclass an existing operator. For example, trying to subclass the ExternalTaskSensor to add a delayed timestamp XCOM output: ```python class ExternalTaskSensorWithCompletionDelay(ExternalTaskSensor): """Return a stable delay target when the upstream task is first observed.""" def poke(self, context): is_done = super().poke(context) delay_target = timezone.utcnow() + timedelta(minutes=30).isoformat() return PokeReturnValue(is_done=bool(is_done), xcom_value=delay_target) ``` The xcom value here will not actually be written, because the parent's class execute method ignores the return value. Is this expected behaviour, or should we always be returning the value of `super().execute()`? -- 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]
