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 acknowledged functionality in deferrable mode for PubSubPullSensor #39711

Merged
merged 1 commit into from
May 21, 2024

Conversation

MaksYermak
Copy link
Contributor

In this PR I have refactored the trigger for PubSubPullSensor. It is needed to fix the issue in acknowledged functionality in deferrable mode. We need to acknowledge after the pull, because the pull command generates an acknowledge ID which expires by default after 10 seconds.


^ 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 May 20, 2024
@eladkal
Copy link
Contributor

eladkal commented May 20, 2024

It is needed to fix the issue in acknowledged functionality in deferrable mode.

If this is a bug fix then we should have a unit test to avoid regression

@MaksYermak
Copy link
Contributor Author

MaksYermak commented May 20, 2024

It is needed to fix the issue in acknowledged functionality in deferrable mode.

If this is a bug fix then we should have a unit test to avoid regression

@eladkal Previously it was a problem in bad trigger design. In that design wasn't included that acknowledge ids exist only 10 seconds after pull. This creates an issue for users when the trigger passes successfully, but on PubSub service in Google Cloud they still have messages. It happens for users, because PubSub's trigger sleep after the pull command and then on the acknowledged stage trigger acknowledges the expired ID. I do not see how we can cover this case by unit test.

@VladaZakharova
Copy link
Contributor

VladaZakharova commented May 21, 2024

Hi @potiuk @Lee-W @Taragolis @ahidalgob !
Can you please take a look on the changes here? Thanks!

@potiuk potiuk merged commit 791f3cf into apache:main May 21, 2024
41 checks passed
RNHTTR pushed a commit to RNHTTR/airflow that referenced this pull request Jun 1, 2024
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.

None yet

5 participants