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 Google tasks integration, with initial read-only To-do list #102629
Conversation
homeassistant/components/google_tasks/application_credentials.py
Outdated
Show resolved
Hide resolved
async def async_get_authorization_server(hass: HomeAssistant) -> AuthorizationServer: | ||
"""Return authorization server.""" | ||
return AuthorizationServer( | ||
authorize_url=OAUTH2_AUTHORIZE, |
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.
Side note: Shared application credentials between integrations came to mind when I read this. I don't remember what we said about this in the architecture discussion.
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.
Yeah, I did as well. We just punted given there weren't many use cases at the time, and the auth methods were different between this. Since then we added many more for google, and some older auth methods were removed. It would be even better if we could make google calendar (the most popular) support web auth, so all creds could be compatible.
I'm happy to consider and design if this is a priorty for core developers to discuss with me (i'm not interested if it takes many months :) )
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.
Following up, #103570 is an attempt to make google calendar support web auth so it could have compatible creds with other google integrations.
async_add_entities( | ||
( | ||
GoogleTaskTodoListEntity( | ||
TaskUpdateCoordinator(hass, api, task_list["id"]), |
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.
Why do we use a coordinator if there's one coordinator per entity?
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.
Yes, agree it is not striclty necessary as there is no sharing of data.
I'm only using it because it seems to be the easist way to handle all the best practices to move up the quality scale (exception handling, logging on exceptions, re-auth handling, etc), and seems nice for separating responsibilities.
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.
Ok.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Proposed change
Add Google tasks integration, with an initial todo platform implementation for a read-only To-do list. The scope is reduced to make the initial PR easier to review, focusing on the skeleton and minimal platform implementation.
Work plan includes the following PRs:
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: