Sensors should be consistent about whether or not they return a value

[SamWheating]

Maybe more of a discussion topic here - but included one example change.

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():

airflow/providers/http/src/airflow/providers/http/sensors/http.py

Lines 160 to 162 in cd3a7e1

	def execute(self, context: Context) -> Any:
	if not self.deferrable or self.response_check:
	return super().execute(context=context)

but most sensors will not, implicitly returning None:

airflow/providers/standard/src/airflow/providers/standard/sensors/filesystem.py

Lines 120 to 122 in 8dd76f1

	def execute(self, context: Context) -> None:
	if not self.deferrable:
	super().execute(context=context)

This makes for a really confusing experience when trying to subclass or extend an existing sensor. For example, trying to subclass the ExternalTaskSensor to add a delayed timestamp XCOM output:

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 and task.execute() returns nothing.

Is this expected behaviour? Or should we always be returning the value of super().execute()? I am open to discussion here but in my mind this sort of thing quietly breaks the extensibility and flexibility of builtin or provided operators.

---

Important

🛠️ Maintainer triage note for @SamWheating · by @potiuk · 2026-08-13 12:55 UTC

Helpful heads-up from the maintainers — please address before this PR can be reviewed:

• ❌ Merge conflicts. See docs.

Full list of what we check: Pull Request quality criteria.

The ball is in your court — you've been assigned to this PR. Fix the above, then mark it Ready for review.

_{Automated triage — may be imperfect; a maintainer takes the next look.}

[SamWheating]

It looks like the one test failure is unrelated / maybe a flakey test

[SamWheating]

cc @potiuk - conflicts have been fixed. I am mostly interested in your thoughts on this issue / the inconsistency of the interface around sensors. Do you think that this is something we should be opinionated on?

[potiuk]

Thanks for raising this. My view: yes, we should be opinionated, but narrowly.

BaseSensorOperator.execute() already has a contract. It returns PokeReturnValue.xcom_value (or None for a plain bool poke()). An execute() override that calls super().execute(context) without returning it breaks that contract for anyone who subclasses the sensor, which is exactly your ExternalTaskSensor example. So the rule should be: if a sensor overrides execute() for the non-deferrable path, it must return super().execute(context).

This is behaviour-neutral for our own sensors. None of the 56 overrides that currently drop the value has a poke() that returns PokeReturnValue, so they still return None and push no XCom. The only visible change is for user subclasses that return PokeReturnValue(xcom_value=...), and for them it fixes a silently dropped value. I'd keep execute_complete() return values for deferrable mode out of scope, since changing those would change XComs people already consume.

Suggested way forward: land this PR as the reference change, then do the remaining sensors as one PR per provider. Optionally add a small prek check that flags a sensor execute() calling super().execute(...) without returning it.

On this PR specifically:

• CI failures are not flaky. When the branch was rebased over #68997, the deferrable branch got poke_interval=self.poll_interval back (lines 485 and 510). poll_interval is now a deprecated property, so the 5 deferrable ExternalTaskSensor tests fail on the prohibited AirflowProviderDeprecationWarning in every compat job. It needs to be self.poke_interval.
• Please keep the diff minimal: just return super().execute(context) in the existing if not self.deferrable: branch, without removing the else: and dedenting the deferrable code. The restructuring is what caused the bad merge, and it hides the one-line change.
• Please change the return annotation from -> None to -> Any.
• Please add a test that fails without the change: a subclass whose poke() returns PokeReturnValue(is_done=True, xcom_value=...), asserting that the non-deferrable execute() returns that value.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting