Add wait_for_termination parameter and fix double-deferral in PowerBIDatasetRefreshOperator#60369
Add wait_for_termination parameter and fix double-deferral in PowerBIDatasetRefreshOperator#60369vincbeck merged 8 commits intoapache:mainfrom
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/powerbi.py
Outdated
Show resolved
Hide resolved
dabla
left a comment
There was a problem hiding this comment.
Congratulations @shreyas-dev, looking good to me beside the remark made by @shahar1 I think where are good nice improvement.
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/powerbi.py
Outdated
Show resolved
Hide resolved
|
Hey @dabla, fixed "imperative mood" docstring test case which was failing. |
|
Hey @vincbeck , Resolved www-hash.txt. |
|
Hey @vincbeck, This test case failure looks unrelated to this PR. I have updated my branch. Can you please check ? |
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/powerbi.py
Outdated
Show resolved
Hide resolved
|
@dabla , Let me know if anything else is needed to get this merged. |
|
Used |
|
Ran |
|
Hey @vincbeck , I'm guessing structlog was introduced in airflow 3+ and airflow 2.11 used standard logging. How to resolve this issue? On the contrary I'm now thinking of removing the warning cause Its a just informational message. logger.warning(
"The PowerBIDatasetRefreshOperator now always uses deferrable execution when wait_for_termination=True."
)In this case I can remove structlog entirely |
Can you check how it is done in the other providers? |
|
Hey @vincbeck, Updated the code. |
providers/microsoft/azure/src/airflow/providers/microsoft/azure/operators/powerbi.py
Outdated
Show resolved
Hide resolved
|
The failed test doesn't look to be related to this PR https://github.com/apache/airflow/actions/runs/21616745897/job/62362457618?pr=60369 |
…o wait_for_completion
|
Hey @vincbeck, Apologies I have fixed the test case and verified all the test cases by running it locally. |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…DatasetRefreshOperator (apache#60369)
…DatasetRefreshOperator (apache#60369)
…DatasetRefreshOperator (apache#60369)
…DatasetRefreshOperator (apache#60369)
Description
This PR adds an optional
wait_for_terminationparameter toPowerBIDatasetRefreshOperatorand refactors the execution flow to fix the double-deferral anti-pattern present in the current implementation.Motivation and Context
New Feature:
Users need the ability to trigger Power BI dataset refreshes without waiting for completion. This is useful for:
Bug Fix:
The current implementation uses an inefficient double-deferral pattern:
execute()→ defer → triggers refresh and returnsdataset_refresh_idget_refresh_status()→ defer again → polls for completionexecute_complete()→ processes final resultThis is unnecessary because
PowerBITrigger.run()already handles both triggering the refresh AND polling for completion in a single async operation whenwait_for_termination=True.Was generative AI tooling used to co-author this PR?