Add token manager - #452
Conversation
|
|
||
| task_a.cancel() | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await task_a |
There was a problem hiding this comment.
This review is incorrect.
|
I've tested this overnight ok. |
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
🤖 Augment PR SummarySummary:
🤖 Was this summary useful? React with 👍 or 👎 |
| access_token: str, | ||
| token_manager: TokenManager, | ||
| timeout: int = DEFAULT_TIMEOUT, | ||
| websession: aiohttp.ClientSession | None = None, |
There was a problem hiding this comment.
tibber/data_api.py:41 This replaces the publicly exported TibberDataAPI(access_token=...) constructor contract with token_manager: positional callers now pass a str that later fails at async_get_access_token, while keyword callers fail at construction. Other locations where this applies: tibber/realtime.py:35.
Severity: medium
Other Locations
tibber/realtime.py:35
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
I don't think the TibberDataAPI is meant to be instantiated on its own. But if we want to make it backwards compatible, I can address this.
| return self._access_token | ||
|
|
||
| def set_access_token(self, access_token: str) -> None: | ||
| """Update the stored access token synchronously.""" |
There was a problem hiding this comment.
tibber/token_manager.py:47 An in-flight async_get_access_token() refresh can complete after set_access_token() and overwrite the manually supplied value, so later HTTP/RT calls silently revert to a stale bearer token. This also affects the still-supported deprecated Tibber.set_access_token() path, which calls this setter.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
The race is only reachable when a refresh_access_token callback is configured and the caller also pushes tokens via set_access_token — which is contradictory usage. Looking at async_get_access_token:
if self._refresh_access_token is None:
return self._access_token # no task, no raceWithout a callback there is never an in-flight refresh task, so set_access_token is fully deterministic. With a callback, the callback is the single source of truth by design — a completing refresh writing the callback's token is the intended outcome, not a stale revert. I've added docstring notes to TokenManager.set_access_token, Tibber.set_access_token, and TibberDataAPI.set_access_token making this contract explicit.
Consolidate the token reference and refresh into a class.
The goal is to avoid having multiple token references when using this library in an application. The existing multiple references are allowing the different references to drift apart. Also make the token refresh code simpler, and also the realtime connection less likely to encounter an expired token.
I need to test the code live for some days, before it could be merged.
closes #448