Skip to content

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!

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.

5 participants