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 YouTube integration #92988
Add YouTube integration #92988
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.
Thanks for the PR. Please provide return types for all functions and methods. We also require tests for the config flow. See the Google Mail example.
Yea working on it, but I thought I'd throw it out for other to give some preliminary feedback :) |
@tkdrob Are you maybe able to help me out with the test for triggering a reauth after setup? I can't get the method to work. |
It is good practice to limit the first PR to the minimum needed to provide the user functionality. Let's leave to reauth step for a followup PR to keep things small. This is only an issue when the user changes the password which likely revokes the oauth authorization. |
What would you suggest, since I took google mail as basis (as you obviously noticed), what's the best way to keep that out of here? |
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.
See below
Do you have an example json response for the unit test? |
This is printed with
|
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.
See below
Co-authored-by: Robert Hillis <tkdrob4390@yahoo.com>
"homeassistant.components.youtube.config_flow.build", | ||
side_effect=HttpError( | ||
Response( | ||
{ |
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.
Optional: I'd suggest extracting constants for these large dicts.
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.
To what place would be the best?
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.
I don't know, maybe a constant somewhere? I don't have a strong opinion about this so feel free to do later or not.
Thanks @joostlek 👍🏼 |
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.
Please address the comments in a new PR. Thanks!
Proposed change
Add YouTube integration. First you login via Google, after which you can select channels from your subscription list. It will fetch the latest video and the amount of subscribers for a youtube channel.
This is a very early draft, please take a look and give some feedback for code and for how it works.
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: