Add S3 Bucket integration for backups #134206
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
home-assistant
bot
added
cla-needed
has-tests
integration: s3backup
new-integration
labels
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
quality-scale
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
| raise ConfigEntryAuthFailed( | ||
| f"Invalid authentication token for S3 account: {err}" | ||
| ) from err | ||
| except Exception as err: # noqa: BLE001 |
There was a problem hiding this comment.
Don't catch all exceptions. Always limit it to errors you expect it to be raised
There was a problem hiding this comment.
HI Paulus! Thank you for reviewing and taking your time. I am new in homeassistant and perhaps would need a little more help. But I am eager to learn. This is the list of exceptions botocore could raise:
https://boto3.amazonaws.com/v1/documentation/api/latest/guide/error-handling.html#botocore-exceptions
I can not see in the documentation of session.client which exceptions it could raise. I browsed through the code and I think that could be a lot. Many should be covered by ClientError. But how should I take care for the Exceptions I am not aware of? I thought the "final" exception is taking care for this. If I would remove it - and an uncatched exceptions is being raised - what happens then? What is your recommendation?
|
Did you think about using one of the asyncio implementations (aiobotocore/aioboto3)? This can make your code significantly shorter (check my azure_storage PR for a comparison) |
…into s3-backup-location
Hi, yes i tried to use the async libs, but did not succeed. My skills have not been high enough to get this sorted out :( |
|
Will this addon work for any s3 compatible storage? That would allow a large number of providers out of the box (some are listed below) and eliminate the need for dedicated provider support such as #134085 and #134014. Non exhaustive list of s3 compatible providers:
|
|
@mbrevda should be the case. Under the hood S3 storage also appeals to a certain group of users. @mkohns do you need an helping hand here? |
imo we should use the async libs (especially if they are already available), because performance will be better |
Breaking change
Proposed change
Inspired by #134014
I added a new integration handling S3 buckets for backup locations.
I tested this with iDrive, Minio and Azure Storage (over S3Proxy)
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: