Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix consistent return response from PubSubPullSensor #42080

Conversation

gopidesupavan
Copy link
Member

closes: #41877


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Sep 7, 2024
@eladkal eladkal requested review from Lee-W and shahar1 September 20, 2024 17:55
@gopidesupavan gopidesupavan force-pushed the fix-consistent-return-response-in-pubsubpullsensor branch from 681799f to 111c632 Compare September 22, 2024 12:09
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for making the immediate modifications.

@Lee-W
Copy link
Member

Lee-W commented Sep 24, 2024

One thing I just think of @eladkal do you think it's a breaking change? as we change the trigger here

@potiuk
Copy link
Member

potiuk commented Oct 2, 2024

One thing I just think of @eladkal do you think it's a breaking change? as we change the trigger here

IMHO we can treat it as a bugfix.

@potiuk
Copy link
Member

potiuk commented Oct 2, 2024

But it would be nice to add a note at the top of the CHANGELOG explaining the change expected.

@Lee-W
Copy link
Member

Lee-W commented Oct 2, 2024

Yep, it sounds more like a bug fix to me, but I would like to be cautious. I'll merge it later today if no one object.

@Lee-W Lee-W merged commit 64e972c into apache:main Oct 2, 2024
55 checks passed
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
* fix consistent return response pubsubsensor

* removed messages_callback argument to pubsub trigger and using it in execute_complete

* updated variable name

* updates as per comments, added return types and refactored logic

* update types, tests and use inherit exception
@gopidesupavan gopidesupavan deleted the fix-consistent-return-response-in-pubsubpullsensor branch November 2, 2024 13:05
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
* fix consistent return response pubsubsensor

* removed messages_callback argument to pubsub trigger and using it in execute_complete

* updated variable name

* updates as per comments, added return types and refactored logic

* update types, tests and use inherit exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent XCom message format when using PubSubPullSensor with deferrable=True
4 participants