New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add reauth flow to ring integration #103758
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great! I have some suggestions about readability improvements and think its best to do the dependency bump first in a separate PR fixing the compatibility issues, then doing reauth in a follow up. (I'll happily review quickly if you reference the bump pr)
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
5f6b8b4
to
852b5a0
Compare
Thanks @allenporter, I have implemented all of your suggestions except for the dependency bump separation due to the reasons given on the other comment and in the PR description. Hopefully it makes sense and I cannot see another way on this occasion without making things very complicated. This is a one off issue as going forwards consumers of ring_doorbell will only catch the library specific exceptions rather than catching lower level implementation exceptions. qq, should I Resolve Conversation myself when I've implemented the suggestion or wait for you to do it? Many thanks! |
My personal approach is:
|
5e9f5f0
to
7a499a5
Compare
3c556a8
to
16b88d8
Compare
16b88d8
to
0069928
Compare
Many thanks @allenporter for all your help with this |
Proposed change
Add reauthorisation flow to the ring integration. Currently token expiry requires users to delete and re-create the ring integration which is undesirable.
In order for this to work the ring_doorbell library that this integration uses has some new error types that need to be caught and handled by the integration. Hence it depended on PR Bump ring_doorbell to 0.8.0 and handle new exceptions which has now been merged.
N.B. I based these changes heavily on PR #103351 so many thanks to @dknowles2 and the reviewers on that PR.
Type of change
Additional information
Checklist
black --fast 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: