Skip to content

Make Pub/Sub return_immediately configurable, deprecate its default - #73504

Open
shahar1 wants to merge 4 commits into
apache:mainfrom
shahar1:pubsub-return-immediately-deprecation
Open

shahar1 wants to merge 4 commits into
apache:mainfrom
shahar1:pubsub-return-immediately-deprecation

Conversation

@shahar1

@shahar1 shahar1 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

AI Summary

Supersedes #67621, which has been idle since 2026-05-27. The original work is by
@michaelpri10 and is preserved here as the first commit, authored to him.

PubSubPullOperator, PubSubPullSensor and PubsubPullTrigger hardcoded
return_immediately=True, which relies on a Pull option Google deprecated because
it can return zero messages while a backlog exists. This makes the option
configurable, keeps True as the default so existing behaviour is preserved, and
warns that the default will flip in the first Google provider major release after
March 31, 2027.

Commits

Commit Author What
Make return_immediately configurable for the Pub/Sub modules @michaelpri10 The original feature, squashed from his 7 commits into his net delta
Warn when return_immediately is unset rather than when it is set @shahar1 Inverts the deprecation trigger; extends it to PubsubPullTrigger; shares one message constant
Set return_immediately in the Pub/Sub examples @shahar1 Example Dags no longer teach the deprecated pattern
Document the Pub/Sub return_immediately changes for users @shahar1 Changelog note and operator guide

What changed relative to #67621

Click here
  • The deprecation now fires when return_immediately is unset, not when it is
    set.
    Warning only on explicit use leaves everyone still on the implicit default
    hearing nothing, and nags users who already made a deliberate choice. This is the
    main decision worth a maintainer's opinion.
  • PubsubPullTrigger is covered too. It was uncovered, yet the google+pubsub
    scheme constructs it directly for asset watchers, so those Dag authors silently got
    the deprecated default. Its message names the subscription, because the trigger is
    built inside MessageQueueTrigger.serialize() where stacklevel=2 resolves to
    common.messaging's file rather than the user's watcher. The operator and sensor
    deliberately do not interpolate it — subscription is a template field there, so
    at __init__ time it can still hold an unrendered Jinja expression, which
    validate-operators-init rejects.
  • Deferrable PubSubPullSensor now respects return_immediately. It previously
    dropped the argument when handing off to the trigger, so it always behaved as
    True. This is a real behaviour change for anyone already using
    deferrable=True, return_immediately=False, and it is called out in the changelog.
  • The warning message lives in one constant rather than three copies, so the
    removal date cannot drift. The warnings.warn call stays in each class:
    fixup_decorator_warning_stack only adjusts the stack for modules that define an
    operator, so moving the call out would break the frame the warning points at.
  • Tests: dropped a module-level filterwarnings mark that silenced every
    return_immediately deprecation in these files, and added assertions that an unset
    argument still resolves to True — the backward-compatibility contract of the
    deprecation, which nothing pinned.

Verification

  • Full Providers[google] suite: 5379 passed, 41 skipped
  • breeze run mypy, breeze build-docs google, prek pre-commit and manual stages: all clean
  • Each commit is green on its own
  • Exercised against live GCP Pub/Sub: operator with return_immediately=False and a
    backlog pulled messages; with True on an empty subscription returned [] without
    hanging; the sensor and async trigger both pulled; an unset construction warned and
    resolved to True

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

michaelpri10 and others added 4 commits September 21, 2026 23:23
The Pub/Sub Pull operator, sensor and trigger hardcoded
return_immediately=True, which relies on a Pull option Google deprecated
because it can return zero messages while a backlog exists. Users had no way
to opt into the long-polling behaviour Google recommends instead.

Keeping True as the default preserves existing behaviour, so the change is
paired with a deprecation warning announcing the coming flip.
Warning only when a user passes the option leaves the people who most need
the notice -- everyone still on the implicit default -- hearing nothing, and
it nags the users who already made a deliberate choice. Inverting it also
lets the argument stay absent from a call without silently taking the
deprecated path.

PubsubPullTrigger gains the same treatment. Nothing covered it before, yet it
is constructed directly by the google+pubsub scheme for asset watchers, so
those Dag authors were getting the deprecated default with nothing telling
them. It names the subscription in its message because it is built inside
MessageQueueTrigger.serialize(), where stacklevel=2 resolves to
common.messaging's file rather than the user's watcher. The operator and the
sensor cannot do the same: subscription is a template field there, so at
__init__ time it can still hold an unrendered Jinja expression.

Only the message text is shared, in one constant. The warn call stays in each
class: fixup_decorator_warning_stack only adjusts the stack for modules that
define an operator, so moving the call out of them would break the frame the
warning points at.

The tests drop a module-level filterwarnings mark that silenced every
return_immediately deprecation in these files, and start asserting that an
unset argument still resolves to True -- the backward-compatibility contract
of the deprecation, which nothing pinned.
MessageQueueTrigger.serialize() builds a PubsubPullTrigger from the keyword
arguments it was given, so a google+pubsub watcher that omits
return_immediately warns during Dag serialization, in the Dag processor
rather than in any task log. The examples are rendered into the operator and
message-queues guides, so they were teaching the pattern the deprecation
steers users away from.
Two things reach users and neither is visible from the release notes
otherwise: the new deprecation warning, which for asset watchers appears in
Dag processor logs where nobody looks for it, and the deferrable sensor
starting to honour an argument it used to drop on the floor. The operator
guide also never said that the operator waits indefinitely in deferrable
mode, which is the behaviour most likely to surprise.
@boring-cyborg boring-cyborg Bot added area:providers kind:documentation provider:google Google (including GCP) related issues labels Sep 21, 2026
@shahar1

shahar1 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

CC: @VladaZakharova @MaksYermak

@aaron-y-chen aaron-y-chen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers kind:documentation provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants