Skip to content

Conversation

@mvfc
Copy link
Contributor

@mvfc mvfc commented Apr 30, 2025

Description

This PR introduces deferrable versions of the Power BI workspace- and dataset-list operators in the Azure provider package. Instead of blocking a worker slot while calling the Power BI REST API, these operators now:

  • Defer execution using custom BaseTrigger implementations
  • Poll asynchronously for the full list of workspaces or datasets
  • Resume execution once data is available and push IDs into XCom

New Hook methods

  • PowerBIHook.get_workspace_list()
  • PowerBIHook.get_dataset_list(group_id: str)

New Trigger classes

  • PowerBIWorkspaceListTrigger
  • PowerBIDatasetListTrigger

New Operator classes

  • PowerBIWorkspaceListOperator
  • PowerBIDatasetListOperator

Unit tests

  • Trigger serialization (serialize())
  • Trigger run logic (success and error paths)
  • Operator deferral (TaskDeferred)
  • Operator resume handling (execute_complete)
  • XCom key/value assertions

Motivation and Context

Long-running or paginated API calls can occupy a worker slot for tens of seconds or more, limiting concurrency and throughput. By deferring:

  1. We free up the worker while waiting on Power BI.
  2. We align with Airflow’s best practices for asynchronous I/O.
  3. We make scaling in high-volume environments more efficient.

This change complements existing synchronous operators; it does not remove or alter any blocking behavior, giving users the choice.

How Has This Been Tested?

  • 🧪 Automated tests under tests/providers/microsoft/azure/ covering all new code paths.
  • 🔄 Manual end-to-end DAG against a real Power BI service principal.
  • ✅ Verified that XCom payloads contain the correct ID lists on resume.
  • ✅ Verified appropriate error handling and retries in failure cases.

Documentation

  • Added howto/operator:PowerBIWorkspaceListOperator and howto/operator:PowerBIDatasetListOperator pages.
  • Updated Azure provider README to mention the new deferrable operators.

Backwards-Incompatible Changes?

No. Existing synchronous PowerBIHook methods and operators remain unchanged and fully supported.

New Dependencies?

None. All new code relies on existing Airflow and Azure provider infrastructure.

@mvfc
Copy link
Contributor Author

mvfc commented Apr 30, 2025

ok i redid the rebase locally and pushed it again, sadly that closed the last PR @jscheffl but thanks for pointing it out, not sure what went wrong there

@potiuk
Copy link
Member

potiuk commented Apr 30, 2025

cc: @ambika-garg -> can you take a look please ?

@mvfc mvfc force-pushed the main branch 4 times, most recently from 1ea5e83 to 2bb40a8 Compare April 30, 2025 12:40
@ambika-garg
Copy link
Contributor

reviewed the code, it looks good to me. Let me know if you'd like me to review anything specific.

@mvfc mvfc force-pushed the main branch 7 times, most recently from dcf03ed to 99cd0a3 Compare April 30, 2025 21:27
@mvfc mvfc force-pushed the main branch 2 times, most recently from 813c6a3 to 3b8cb8c Compare May 1, 2025 08:12
@mvfc
Copy link
Contributor Author

mvfc commented May 1, 2025

fixed the db test issues, was passing an extra argument that i removed from the examples in system tests

@mvfc mvfc requested a review from jscheffl May 1, 2025 08:14
@gopidesupavan
Copy link
Member

@mvfc overall LGTM, few nits.

@mvfc
Copy link
Contributor Author

mvfc commented May 1, 2025

@mvfc overall LGTM, few nits.

thanks for the review and comments, will resolve asap :-)

@mvfc mvfc force-pushed the main branch 7 times, most recently from 08c5455 to fee8cb2 Compare May 2, 2025 09:23
@mvfc
Copy link
Contributor Author

mvfc commented May 2, 2025

all resolved, all tests passed

@jscheffl
Copy link
Contributor

jscheffl commented May 2, 2025

@ambika-garg uff there had been many changes, can you re-review? I am not an expert but before merge would like to have some expert view on this. Due to re-base I can not see the DIFF to before :-(

@mvfc
Copy link
Contributor Author

mvfc commented May 2, 2025

@jscheffl

@ambika-garg uff there had been many changes, can you re-review? I am not an expert but before merge would like to have some expert view on this. Due to re-base I can not see the DIFF to before :-(

my bad, still new and getting used to this rebase thing

@gopidesupavan
Copy link
Member

Just two nits, rest LGTM.

@mvfc mvfc force-pushed the main branch 2 times, most recently from a96e90a to 2b13400 Compare May 2, 2025 14:21
mvfc added 2 commits May 2, 2025 21:18
…ors and triggers

- Introduce PowerBIWorkspaceListOperator and PowerBIDatasetListOperator that defer using triggers
- Implement PowerBIWorkspaceListTrigger and PowerBIDatasetListTrigger for async polling of list endpoints
- Hook into the Microsoft Graph API via new async methods on PowerBIHook (get_workspace_list, get_dataset_list)
- Add XCom support in execute_complete callbacks for both operators
- Include unit tests covering deferral, trigger serialization, run logic, and operator resume paths
- Update docs: operator HOWTO for PowerBI workspace/dataset listing
Copy link
Member

@gopidesupavan gopidesupavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, thanks for the changes.

@mvfc
Copy link
Contributor Author

mvfc commented May 2, 2025

Good work, thanks for the changes.

Thanks. Thanks for the help and sharp eye.

@potiuk potiuk merged commit 68737b8 into apache:main May 3, 2025
68 checks passed
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.

5 participants