-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Merge BigQueryTableExistencePartitionAsyncSensor into BigQueryTableExistencePartitionSensor #30231
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
Conversation
…ableExistencePartitionAsyncSensor to BigQueryTableExistencePartitionSensor
…itionSensor when its deferrable attribute is set to True
…itionSensor deferrable mode and BigQueryTableExistencePartitionAsyncSensor deprecation
…r sync and async mode in BigQueryTableExistencePartitionSensor
78aa648 to
7ec1131
Compare
| if deferrable and "poke_interval" not in kwargs: | ||
| kwargs["poke_interval"] = 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t need to be done in this PR but should be a good idea to define 5 as a constant somewhere (maybe BaseSensorOperator.DEFERRABLE_POLL_INTERVAL) so we don’t repeat the magic number everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed AzureDataFactoryPipelineRunStatusAsyncSensor using 60 instead of 5. Do you think we still should add this constant in this case?
Why making this change?
apache-airflow-providers-google treats the deferred execution of its operators and sensors differently, which might cause confusion. For example, BigQueryCheckOperator uses the deferrable parameter to toggle deferred execution while we need to use another sensor for BigQueryTablePartitionExistenceSensor (i.e., BigQueryTablePartitionExistenceAsyncSensor) to achieve the same thing.
What's changed?
In this pull request, I move the deferred logic from BigQueryTablePartitionExistenceAsyncSensor to BigQueryTablePartitionExistenceSensor so that we can keep the consistency and reduce maintenance effort. Instead of removing BigQueryTablePartitionExistenceAsyncSensor, I add a deprecation warning in case users're using it.
^ 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.rstor{issue_number}.significant.rst, in newsfragments.