Skip to content

Conversation

@Lee-W
Copy link
Member

@Lee-W Lee-W commented Mar 22, 2023

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.rst or {issue_number}.significant.rst, in newsfragments.

Lee-W added 2 commits March 22, 2023 09:37
…ableExistencePartitionAsyncSensor to BigQueryTableExistencePartitionSensor
…itionSensor when its deferrable attribute is set to True
@Lee-W Lee-W changed the title Merge BigQueryTableExistencePartitionAsyncSensor intoBigQueryTableExistencePartitionSensor Merge BigQueryTableExistencePartitionAsyncSensor into BigQueryTableExistencePartitionSensor Mar 22, 2023
Lee-W added 2 commits March 22, 2023 17:27
…itionSensor deferrable mode and BigQueryTableExistencePartitionAsyncSensor deprecation
…r sync and async mode in BigQueryTableExistencePartitionSensor
@Lee-W Lee-W force-pushed the merge-BigQueryTableExistencePartitionAsyncSensor-into-BigQueryTableExistencePartitionSensor branch from 78aa648 to 7ec1131 Compare March 22, 2023 08:28
Comment on lines +153 to +154
if deferrable and "poke_interval" not in kwargs:
kwargs["poke_interval"] = 5
Copy link
Member

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.

Copy link
Member Author

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?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants