-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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 Drive integration for backup #134576
base: dev
Are you sure you want to change the base?
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.
Works as expected and LGTM. Missing test coverage should be a priority on follow-up PR.
LGTM - as this uses the folder ID, I'm assuming there could be a chance that someone might "accidentally" delete the folder? Does this impact integration startup / backup service? Could a backup upload "silently" fail for example? Very minor in the grand scheme of things, but I know Google's storage manager likes to recommend things to delete. |
Done. If the folder is deleted the setup fails with: |
Later this could be changed to use repair instead of reconfigure flow. |
So that means that Google Drive will see the difference, it just might be confusing for the end user when logging into their own Google Drive and seeing 2 folders with the same name - IF (and only IF) one is using the Google Drive add-on already |
Maybe just use "Home Assistant/Backups"? |
The name is intentionally just Home Assistant. In the future this integration could expose an action to upload any file to Google Drive, e.g. to upload a camera snapshot or recording, similar to the service in the Google Photos integration. |
I tried this in my prod instance yesterday and it didn't work. The addons property was hitting the 124 bytes per property limit imposed by Google Drive. I just changed the code to use a backups.json file in Google Drive that is updated with a lock from this integration when a new backup is uploaded or an existing one is deleted. |
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Okay So I discussed this in the core team yesterday, and we stand by the rule that communication specific details are stored in a library hosted on Pypi. Even for simple integrations we require a library. There is also the possibility to add more functionality to the integration in the future, like storage percentage. Hence the integration being a backup agent in its current form doesn't really form an exception. And as always, if you need help with this, feel free to reach out. I would love to see this PR succeed! |
It should be made a bronze requirement then. Do you recommend putting the whole DriveClient class in api.py in a library? If yes, that would make it very specific to HA and not comply with the recommendations in https://developers.home-assistant.io/docs/api_lib_data_models/ specifically: "API libraries should try to do as little as possible." The alternative is to add simple wrappers that just make the REST calls. That would be similar to if I was using google-api-python-client (e.g. used by Google Mail) or aiogoogle (used in an earlier commit here). E.g.
|
In theory everything that is specific for the service. The way you implement that is your choice. If I imagine a library for this, it would be abstract. I want to put in an access token (or put in a function which can get me an up to date token) and I want to have methods like async def create_folder(self, name: str, properties: dict[str, str]) -> None: And that is a bit of beauty of this abstraction is that I don't really care how it creates that folder, as long as it does it. This does still leave the logic on how the integration interacts with Google Drive in the integration, but it puts the code on how to do that technically away. I personally really like a typed library, so I would probably use mashumaro to create dataclasses for the return types. |
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.
Actually, the issue when the folder is missing (after config flow) was pretty nice imo. If that folder went missing that definitely warrants an issue the user needs to take a look at (we can still fix it automatically in a RepairFlow later)
Proposed change
Add Google Drive integration. Currently it only provides a backup agent that works with the Home Assistant backup solution introduced in Home Assistant 2025.1.
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
.To help with the load of incoming pull requests: