Support initializing PowerBiClient with an existing access token - #13
wakinniranye31 wants to merge 1 commit into
Conversation
|
Hey @wakinniranye31, thanks for putting this together, the use case is totally valid and I appreciate the detailed write-up. Before we merge though I'd like to ask for a few changes:
Happy to talk through any of this if it's unclear, especially the |
PowerBiClient and PowerBiAuth now accept a token_provider argument (a TokenProvider instance) instead of a raw access_token string. client_id/client_secret/redirect_uri/scope become optional and the whole MSAL/OAuth login() flow is skipped when a provider is supplied - there's nothing left to authenticate. Every request now goes through PowerBiAuth.get_token(), which either delegates to the external TokenProvider or (for the existing MSAL flow) validates/refreshes as before and returns the current token. PowerBiSession.build_headers() calls this once per request rather than reading a cached string, so a custom TokenProvider can rotate or refresh its token transparently - the library never assumes the token it saw last time is still valid. Includes a built-in StaticTokenProvider for the common case of already having a plain token string with no rotation logic needed. Same pattern used by the Azure SDK and Google Auth libraries. Addresses review feedback on this PR: - Replaced the raw access_token: str parameter with a TokenProvider interface (this commit). - New parameters are typed str | None = None / list[str] | None = None (project targets Python 3.10+). - PowerBiAuth.__init__ now raises the same ValueError guard as PowerBiClient.__init__ when constructed directly with neither a token_provider nor a full set of credentials. - samples/use_client_with_access_token.py no longer uses a JWT-shaped placeholder; replaced with an unambiguous 'REPLACE_WITH_YOUR_ACCESS_TOKEN' string. Rebased onto upstream's token-expiration handling (f6ca626) and access-token-validation-before-request (8cc7163) changes, which build_headers()/get_token() now compose with rather than duplicate. Tests: added TestPowerBiClientWithTokenProvider (including a RotatingTokenProvider case proving get_token() is called fresh each time, not cached) and TestPowerBiAuthWithTokenProvider covering the direct-construction guard. Full suite: 135 passed.
331ca36 to
876a702
Compare
|
Thanks for the detailed review, @areed1192 — all four addressed:
One extra thing worth flagging: while rebasing onto Ran the full suite locally after rebasing: 135 passed. Also ran Happy to adjust the |
Closes #8.
Problem
There was no way to construct a
PowerBiClientfrom a token acquired outside this library (e.g. via a service principal client-credentials flow, a managed identity, or an app that already handles its own auth). The only path was the interactive/confidential-client MSAL flow baked intoPowerBiAuth.Change
PowerBiClient.__init__andPowerBiAuth.__init__gain an optionalaccess_tokenparameter.client_id,client_secret,redirect_uri, andscopeare now optional too — eitheraccess_tokenor all four of those must be supplied, enforced with aValueErrorat construction time.access_tokenis provided,PowerBiAuthskips creating themsal.ConfidentialClientApplicationentirely (self.client_app = None), andlogin()becomes a no-op, since there's nothing left to authenticate.PowerBiSession.build_headers(), all the service classes) already only readsself.client.access_token, so no other changes were needed to make requests work.Testing
Added
TestPowerBiClientWithAccessTokenintests/test_client.py:access_tokenskips the MSAL app and stores the token directly,login()afterward is a no-op and doesn't touch the token,access_tokennor full credentials raisesValueError.Ran the full suite locally:
python -m unittest discover -s tests -v→ 21 passed (18 existing + 3 new), all green.Checklist (per CONTRIBUTING.md)
CHANGELOG.mdupdated under[Unreleased]samples/use_client_with_access_token.py)Note on #7
I initially looked at #7 (add a
DateColumnDataTypesmember) intending to fix both in one pass, but the Power BI push-dataset REST API only documentsDateTimefor temporal columns — there's no separateDatetype in the Column object schema, and sibling librarypbipy's equivalent enum doesn't have one either. Adding aDATE = "Date"member would let users build a column the service will likely reject, so I left that one alone and focused this PR on #8 instead, which is a real gap. Happy to comment on #7 with these findings if useful.