josh-fell commented on code in PR #30227:
URL: https://github.com/apache/airflow/pull/30227#discussion_r1149966004
##########
airflow/providers/dbt/cloud/sensors/dbt.py:
##########
@@ -49,13 +51,30 @@ def __init__(
dbt_cloud_conn_id: str = DbtCloudHook.default_conn_name,
run_id: int,
account_id: int | None = None,
+ deferrable: bool = False,
**kwargs,
) -> None:
+ if deferrable and "poke_interval" not in kwargs:
+ # TODO: Remove once deprecated
+ if "polling_interval" in kwargs:
+ kwargs["poke_interval"] = kwargs["polling_interval"]
+ warnings.warn(
+ "Argument `poll_interval` is deprecated and will be
removed "
+ "in a future release. Please use `poke_interval`
instead.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ else:
+ kwargs["timeout"] = 60 * 60 * 24 * 7
Review Comment:
Should there be a check to make sure `timeout` is not in `kwargs` as well
_just in case_ users happen to specify it explicitly while running the sensor
in deferrable mode?
##########
tests/system/providers/dbt/cloud/example_dbt_cloud.py:
##########
@@ -77,6 +77,12 @@
)
# [END howto_operator_dbt_cloud_run_job_sensor]
+ # [START howto_operator_dbt_cloud_run_job_sensor_defered]
+ job_run_sensor_defered = DbtCloudJobRunSensor(
+ task_id="job_run_sensor_defered", run_id=trigger_job_run2.output,
timeout=20
Review Comment:
```suggestion
task_id="job_run_sensor_defered", run_id=trigger_job_run2.output,
deferrable=True, timeout=20
```
Since this example is showcasing the deferrable functionality, let's add the
flag.
##########
docs/apache-airflow-providers-dbt-cloud/operators.rst:
##########
@@ -108,6 +115,7 @@ Use the
:class:`~airflow.providers.dbt.cloud.sensors.dbt.DbtCloudJobRunAsyncSens
status of a dbt Cloud job run asynchronously. This sensor will free up the
worker slots since
polling for job status happens on the Airflow triggerer, leading to efficient
utilization
of resources within Airflow.
+:class:`~airflow.providers.dbt.cloud.sensors.dbt.DbtCloudJobRunAsyncSensor` is
deprecated and will be removed in a future release. Please use
:class:`~airflow.providers.dbt.cloud.sensors.dbt.DbtCloudJobRunSensor` and use
the deferrable mode in that operator.
Review Comment:
Would you mind including this note in a `.. note::` directive? Let's remove
the code snippet example below too since that example shows a deprecated sensor.
##########
docs/apache-airflow-providers-dbt-cloud/operators.rst:
##########
@@ -69,6 +69,14 @@ referenced within the ``default_args`` of the example DAG.
:start-after: [START howto_operator_dbt_cloud_run_job]
:end-before: [END howto_operator_dbt_cloud_run_job]
+Also you can use deferrable mode in this operator if you would like to free up
the worker slots while the sensor is running.
Review Comment:
```suggestion
Also you can use deferrable mode in this sensor if you would like to free up
the worker slots while the sensor is running.
```
Also, this should live in the sensor section rather than the
DbtCloudRunJobOperator section.
##########
tests/system/providers/dbt/cloud/example_dbt_cloud.py:
##########
@@ -77,6 +77,12 @@
)
# [END howto_operator_dbt_cloud_run_job_sensor]
+ # [START howto_operator_dbt_cloud_run_job_sensor_defered]
+ job_run_sensor_defered = DbtCloudJobRunSensor(
+ task_id="job_run_sensor_defered", run_id=trigger_job_run2.output,
timeout=20
+ )
+ # [END howto_operator_dbt_cloud_run_job_sensor_defered]
+
# [START howto_operator_dbt_cloud_run_job_async_sensor]
job_run_async_sensor = DbtCloudJobRunAsyncSensor(
task_id="job_run_async_sensor", run_id=trigger_job_run2.output,
timeout=20
Review Comment:
This sensor should be removed from the system test now that it's deprecated
IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
