Fix Twitch auth token refresh#112833
Conversation
home-assistant
Bot
added
bugfix
cla-signed
integration: twitch
small-pr
|
Hey there @joostlek, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| await self._client.set_user_authentication( | ||
| self._session.token["access_token"], | ||
| OAUTH_SCOPES, | ||
| self._session.token["refresh_token"], | ||
| ) |
There was a problem hiding this comment.
Hmm, I just checked the lib, and apparently when calling this the lib does another call to check if the token is valid. Not sure if I fully like the extra request. Any thought on that?
There was a problem hiding this comment.
I don't like it either, as it's a workaround for the fact that the current version of the lib does not raise an exception when the token is expired. It is a confirmed bug - Teekeks/pyTwitchAPI#293 (comment).
I did not feel comfortable to use the AttributeError exception, especially sine it is thrown in an unrelated part of the code.
When that problem is fixed the correct solution would be retry logic with the refresh.
Got a better idea?
There was a problem hiding this comment.
I just checked the code for set_user_authentication, and the solution was simple. I'll just add a False parameter and it will not make any calls.
await self._client.set_user_authentication(
self._session.token["access_token"],
OAUTH_SCOPES,
self._session.token["refresh_token"],
False,
)
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
There's a lot more to improve, but to get the integration to a working state, this is perfect
frenck
left a comment
frenck
left a comment
There was a problem hiding this comment.
The CI is failing, can you please take a look?
Thanks! 👍
../Frenck
frenck
left a comment
frenck
left a comment
There was a problem hiding this comment.
Trying again using the correct review state 🙈
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
frenck
added
the
smash
* Fix for expired token * Add auth token refresh. * Eliminate extra auth call * Fixed mock client --------- Co-authored-by: Jonny Bergdahl <bergdahl@users.noreply.github.com>
6 participants
Breaking change
Proposed change
This is a fix for 111730 - Twitch integration spamming the log..
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: