dabla commented on code in PR #60369:
URL: https://github.com/apache/airflow/pull/60369#discussion_r2708669587
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/powerbi.py:
##########
@@ -65,6 +65,7 @@ class PowerBIDatasetRefreshOperator(BaseOperator):
:param check_interval: Number of seconds to wait before rechecking the
refresh status.
:param request_body: Additional arguments to pass to the request body, as
described in
https://learn.microsoft.com/en-us/rest/api/power-bi/datasets/refresh-dataset-in-group#request-body.
+ :param wait_for_termination: If True, wait for the dataset refresh to
complete. If False, trigger the refresh and return immediately without waiting.
Review Comment:
The reason why I would remove the blocking part entirely is that it’s not a
good practise in Airflow. Polling should be handled by triggers, as that’s what
they are designed for, because otherwise it means you block a worker slot in
Airflow, which in reality does nothing except waiting for a response, thus in
my opinion remove the blocking part and replace it purely with deferrable (e.g.
Polling in trigger) is a much better solution imho. Therefor I would deprecrate
the deferrable parameter, log a warning it’s not used anymore and just do
deferred. WDYT @eladkal ?
--
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]
