IDrive e2 backup provider by patrickvorgers · Pull Request #144910 · home-assistant/core

patrickvorgers · 2025-05-14T17:40:41Z

Proposed change

Based on the feedback earlier that S3 backup providers should be specific. I started developing the IDrive e2 backup provider based on the AWS S3 backup integration. This implementation for instance uses a specific config_flow for IDrive e2 which automatically determines the endpoint based on the provided credentials and provides the user a dropdown selection box to select the bucket. The iDrive E2 backup provider has been running for the last month on my production HA machine with no issues.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: #39042
Link to developer documentation pull request:
Link to frontend pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

joostlek · 2025-07-15T20:29:40Z

+    # NOTE: We use boto3 instead of aiobotocore.AioSession because AioSession
+    # causes blocking operations inside the event loop.
+    # Two examples include:
+    # - os.listdir
+    # - ssl.SSLContext.load_verify_locations
+    # These lead to 'Detected blocking call` warnings. Using boto3 inside
+    # async_add_executor_job avoids these issues by running blocking code off the event loop.


Don't other integrations do the same?

Sorry for the late response but I was on holiday 😄

The AWS S3 implementation uses the aiobotocore.AioSession and because I reused that code I ran into this issue. I will raise an issue for the AWS S3 integration describing the issue, close?

I believe there are also ways to load something async in the executor, but I am not 100% sure on that

Don't know either, my suggestion would be to leave it like this.

with all the similarities (and the open discussion about how to avoid it), I don't think we should use different libraries.
Something like

client = hass.async_add_executor_job( session.create_client( service_name="s3", endpoint_url=data.get(CONF_ENDPOINT_URL), aws_secret_access_key=data[CONF_SECRET_ACCESS_KEY], aws_access_key_id=data[CONF_ACCESS_KEY_ID], ) ) await client.__aenter__() # pylint: disable-next=unnecessary-dunder-call

should get rid of the blocking call warning (and ideally it gets fixed in the library afterwards)

Nice, glad to see there’s already an issue and that it’s getting some attention again, very happy you jumped in there.

I totally understand the concern about ending up with two different S3 approaches, and I’m aligned on aiming for a single, non-blocking pattern (ideally via aiobotocore once the blocking bits are sorted out).

From my side I’m happy to:

keep an eye on the aiobotocore issue and its timeline, and

prepare a switch for this integration to the agreed aiobotocore pattern if it looks like the fix will land soon.

If you and the team feel it makes more sense to hold this PR until that’s clearer, I’m OK with that; if not, I’m also fine to get this in now and follow up with a switch once the upstream situation is resolved.

I’ve updated this PR to use aiobotocore, so it aligns with the existing aws_s3 backup integration approach in Core.

For next steps, I’m happy to follow whatever the Home Assistant maintainers prefer. We can either merge this as-is, or hold off until there’s more clarity on the upstream preload proposal (aio-libs/aiobotocore#1462). That PR still needs review (@bdraco / @thehesiod / @webknjaz).

Please let me know how you’d like to proceed.

I'm not really quallified to review the aiobotocore use — I'm mostly enabling people to maintain it (and many other things under @aio-libs), but not intimately familiar with it specifically. Maybe, @bdraco would have a better context to look into it.

@patrickvorgers we're waiting on feedback on aio-libs/aiobotocore#1451 to see which strategy people prefer

@thehesiod, I saw that the "warmup" functionality is now included in the latest release and that partly resolves the issue (see Unblocking creation of SSL context. I have tested that with the idrive_e2 integration

The preference is to have aiobotocore completely async but as far as I understand it, that is not possible in the shortterm. The possibility for specifying an executor as described in your PR, would that enable us to resolve the Unblocking creation of SSL context issue and how would that work?

Maybe @zweckj can also help with the test cases as suggested by @bdraco.

joostlek · 2025-07-15T20:31:53Z

+async def async_setup_entry(hass: HomeAssistant, entry: IDriveE2ConfigEntry) -> bool:
+    """Set up IDrive e2 from a config entry."""
+
+    data = cast(dict, entry.data)


why do we cast this?

It is there for stricter type checker hints. If needed I can also for instance directly use entry.data[CONF_ENDPOINT_URL]. The orignal AWS S3 integration also uses this construct.

I think that is fine to use without casting

Removed the cast

This was fixed in commit

joostlek · 2025-07-15T20:32:17Z

+    except ParamValidationError as err:
+        if "Invalid bucket name" in str(err):
+            raise ConfigEntryError(
+                translation_domain=DOMAIN,
+                translation_key="invalid_bucket_name",
+            ) from err


but if this is not the case, what should we do then?

Basically we could request the user to recreate the bucket. During configuration of the integration the bucket existed (user selected the bucket in the configflow) but now it doesn't anymore.
So instead of the current message: "Invalid bucket name" we could change the message to: "Bucket %s doesn't exist, please recreate the bucket."

This was fixed in commit

joostlek · 2025-07-15T20:32:46Z

+
+async def async_unload_entry(hass: HomeAssistant, entry: IDriveE2ConfigEntry) -> bool:
+    """Unload a config entry."""
+    client: boto3.client = entry.runtime_data


you don't need to retype the runtime data, as that is already part of the typeslot in IDriveE2ConfigEntry

Changed it into: client = entry.runtime_data

This was fixed in commit

joostlek · 2025-07-15T20:33:42Z

+def _build_user_step_schema(
+    access_key: str | None = None, secret_key: str | None = None
+) -> vol.Schema:
+    """Build the user step schema."""
+    if access_key is None or secret_key is None:
+        return STEP_USER_DATA_SCHEMA
+
+    # Return a schema with prefilled values
+    return vol.Schema(
+        {
+            vol.Required(
+                CONF_ACCESS_KEY_ID,
+                default=access_key,
+            ): cv.string,
+            vol.Required(
+                CONF_SECRET_ACCESS_KEY,
+                default=secret_key,
+            ): cv.string,
+        }
+    )


There's a function for this in the ConfigFlow, something like self.add_suggested_values_to_schema or something along those lines

This was fixed in commit

joostlek · 2025-07-15T20:34:18Z

+                resp = await http.post(
+                    GET_REGION_ENDPOINT_URL,
+                    json={"access_key": user_input[CONF_ACCESS_KEY_ID]},
+                )
+                resp.raise_for_status()
+                payload = await resp.json()
+                if payload.get("resp_code") == GET_REGION_INVALID_CREDENTIALS_CODE:
+                    _raise_invalid_credentials(resp)


This is device or service specific code and should exist in a library hosted on Pypi

This was fixed in commit

home-assistant · 2025-07-15T20:34:41Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

joostlek · 2025-09-12T09:27:19Z

+    # NOTE: We use boto3 instead of aiobotocore.AioSession because AioSession
+    # causes blocking operations inside the event loop.
+    # Two examples include:
+    # - os.listdir
+    # - ssl.SSLContext.load_verify_locations
+    # These lead to 'Detected blocking call` warnings. Using boto3 inside
+    # async_add_executor_job avoids these issues by running blocking code off the event loop.


I believe there are also ways to load something async in the executor, but I am not 100% sure on that

joostlek · 2025-09-12T09:27:34Z

+async def async_setup_entry(hass: HomeAssistant, entry: IDriveE2ConfigEntry) -> bool:
+    """Set up IDrive e2 from a config entry."""
+
+    data = cast(dict, entry.data)


I think that is fine to use without casting

joostlek · 2025-09-12T09:28:57Z

+        # Delete both the backup file and its metadata file
+        await self._hass.async_add_executor_job(
+            functools.partial(
+                self._client.delete_object, Bucket=self._bucket, Key=tar_filename
+            )
+        )
+        await self._hass.async_add_executor_job(
+            functools.partial(
+                self._client.delete_object, Bucket=self._bucket, Key=metadata_filename
+            )
+        )


Switching from the executor to the event loop is expensive, so ideally we don't switch often, can we group these?

Yes can be grouped into one delete_objects call

This was fixed in commit

joostlek · 2025-09-12T09:29:39Z

+
+        return self.async_show_form(
+            step_id="user",
+            data_schema=self.add_suggested_values_to_schema(STEP_USER_DATA_SCHEMA, {}),


We don't have to call to add suggested values to schema if the dict we use to fill it with is empty

Agreed, is an unnecessary call

This was fixed in commit

joostlek · 2025-09-12T09:43:21Z

+        try:
+            # Information should be available from the previous step
+            assert self._endpoint_url is not None
+            assert self._access_key is not None
+            assert self._secret_key is not None
+            # List buckets using the provided credentials
+            buckets = await self.hass.async_add_executor_job(
+                _list_buckets,
+                self._endpoint_url,
+                self._access_key,
+                self._secret_key,
+            )
+        except ClientError:


Only have things in the try block that can raise

Moved the assertion outside of the try-block

This was fixed in commit

joostlek · 2025-09-12T09:43:54Z

+            # Go back to the user step if there are errors getting buckets
+            # Prefill the access key and secret key fields with the current values


would it make sense to have the user step responsible for fetching the buckets?

I’d suggest keeping the _list_buckets call in the bucket step rather than the user step. Each step in a config flow should manage its own data and initialization: the user step is responsible for collecting credentials, while the bucket step is responsible for selecting a bucket. Having the bucket step fetch the list keeps responsibilities clear, avoids mixing concerns, and makes the flow easier to maintain.

Sure, but I would argue that validating the input is a part of the concern of the user step. I don't expect the bucket list to change at this point so we can just fetch it once and display it here right?

Hopefully I interpreted your remark/question correctly.

You are correct that the bucket list itself doesn't change in between steps. The would be one edge case where the user deletes the bucket while inbetween steps.

The bucket fetch is done based on the provided credentials. In theory it is possible that the provided access-key is correct but no buckets are assigned to this access-key. While creating the access-key the user is forced to assign one or more buckets to this access-key but is free to remove that bucket afterwards. In that case you would have an access-key with no buckets.

You cannot add buckets to existing access-keys so it is usefull for the user to be presented again with the credential screen so he/she can provide new access-key information, when there are no buckets associated with the provided access-key. In case of a hiccup in the _list_buckets call, the user can use the existing credentials and try again.

joostlek · 2025-09-12T09:44:44Z

+          "bucket": "Bucket name"
+        },
+        "data_description": {
+          "bucket": "Bucket must be writable by the provided credentials."


The data description should explain the data that is needed, not give extra hints on how the user should set up the integration.

Agree and changed the description.

This was fixed in commit

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

emontnemery · 2026-02-11T10:21:40Z

(Close + reopen to restart CI against current tip of dev)

joostlek · 2026-02-11T11:05:29Z

+    cm: Any | None = None
+    client: S3Client | None = None


Why are these | None I would consider that we'd know the type of cm and that we don't have to type it Any

Good point, cm is gone now so no Any type needed. client is still | None because it’s created inside the try and may not exist if create_client().__aenter__() fails, It is used in the exception handling in the _async_safe_client_close.

Fixed in this commit

joostlek · 2026-02-11T11:08:32Z

+    with patch(
+        "aiobotocore.session.AioSession.create_client",
+        autospec=True,
+        return_value=AsyncMock(),
+    ) as create_client:


Ideally we patch the library where we use it, so homeassistant.components.idrive_e2..

Okay, I’ll patch AioSession.create_client at the Home Assistant integration import path (homeassistant.components.idrive_e2...) instead of the library path to match where it’s used.

Fixed in this commit

joostlek · 2026-02-11T11:08:53Z

+@pytest.fixture
+def mock_aiohttp_session() -> Generator[AsyncMock]:
+    """Patch async_get_clientsession to return a mocked aiohttp session."""
+    mock_session = AsyncMock()
+    with patch(
+        "homeassistant.components.idrive_e2.config_flow.async_get_clientsession",
+        return_value=mock_session,
+    ):
+        yield mock_session


We don't have to mock this one really

Agreed and has been removed.

Fixed in this commit

joostlek · 2026-02-11T11:09:23Z

+    mock_client.list_buckets.return_value = {
+        "Buckets": [{"Name": USER_INPUT[CONF_BUCKET]}]
+    }


Why isn't this the default in the mock? You'll likely use it in more tests

I have moved the default list_buckets return value into the shared mock_client fixture and drop the repeated assignments in the tests.

Fixed in this commit

joostlek · 2026-02-11T11:10:05Z

+    mock_client.list_buckets.return_value = {
+        "Buckets": [{"Name": USER_INPUT[CONF_BUCKET]}]
+    }


In the case you do followup on the last comment, if this is set as default, you don't have to set it again when removing a side_effect

Removed assignment because of previous fix.

Fixed in this commit

joostlek · 2026-02-11T11:10:46Z

+    MockConfigEntry(
+        domain=DOMAIN,
+        data={
+            CONF_BUCKET: USER_INPUT[CONF_BUCKET],
+            CONF_ENDPOINT_URL: USER_INPUT[CONF_ENDPOINT_URL],
+        },
+        unique_id="existing",
+    ).add_to_hass(hass)


Why don't you use mock_config_entry fixture

I updated the code and now use the mock_config_entry as a template because MockConfigEntry.data is immutable.

Fixed in this commit

joostlek · 2026-02-11T11:11:40Z

+    # If close() masks the original error, this would raise RuntimeError instead
+    with pytest.raises(ConfigEntryError):
+        await async_setup_entry(hass, mock_config_entry)


So we shouldn't touch async_setup_entry directly, instead we should use await hass.config_entries.async_setup() and then we can assert that the mock_config_entry.state isSETUP_ERROR

I switched the test to set up the entry via hass.config_entries.async_setup().

Fixed in this commit

joostlek · 2026-02-11T11:12:08Z

+    with patch(
+        "aiobotocore.session.AioSession.create_client",
+        side_effect=exception,
+    ):


patch where we use it, and preferrably use the shared mock

Adopted patch where used

Fixed in this commit

joostlek · 2026-02-11T11:12:26Z

+async def test_setup_entry_head_bucket_error(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_client: AsyncMock,
+) -> None:
+    """Test setup_entry error when calling head_bucket."""
+    mock_client.head_bucket.side_effect = ClientError(
+        error_response={"Error": {"Code": "InvalidAccessKeyId"}},
+        operation_name="head_bucket",
+    )
+    await setup_integration(hass, mock_config_entry)
+    assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR
+
+
+async def test_setup_entry_head_bucket_not_found_error(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_client: AsyncMock,
+) -> None:
+    """Test setup_entry error when head_bucket returns: Not Found."""
+    mock_client.head_bucket.side_effect = ClientError(
+        error_response={"Error": {"Code": "404", "Message": "Not Found"}},
+        operation_name="head_bucket",
+    )
+    await setup_integration(hass, mock_config_entry)
+    assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR


This can be parametrized

Agreed, these head_bucket error tests need to be parametrized.

Fixed in this commit

zweckj

It'd be nice to also get the improved buffer handling to the other S3 integrations if you don’t mind

Fixed

zweckj · 2026-02-12T23:48:56Z

Please DM your Discord user, so we can add you to the appropriate channels, thanks!

patrickvorgers · 2026-02-13T21:03:31Z

It'd be nice to also get the improved buffer handling to the other S3 integrations if you don’t mind

Created PR #162955 for S3
Created PR #162958 for Cloudfare R2

patrickvorgers added 4 commits May 12, 2025 20:59

Implementation based on s3_aws with custom config_flow

cdfb958

Merge branch 'home-assistant:dev' into idrive-e2-backup-provider

120a3d6

Do not abort flow on bucket error but go back to user step

dbdf908

Adjusted manifest

863d53f

home-assistant Bot added cla-signed has-tests new-integration integration: idrive_e2 quality-scale labels May 14, 2025

patrickvorgers changed the title ~~Idrive e2 backup provider~~ IDrive e2 backup provider May 14, 2025

patrickvorgers mentioned this pull request May 14, 2025

Images for the new IDrive e2 backup core integration home-assistant/brands#7065

Merged

13 tasks

c0ffeeca7 mentioned this pull request May 15, 2025

IDrive e2 backup documentation home-assistant/home-assistant.io#39029

Closed

8 tasks

patrickvorgers mentioned this pull request May 15, 2025

IDrive e2 backup documentation home-assistant/home-assistant.io#39042

Closed

8 tasks

Merge branch 'dev' into idrive-e2-backup-provider

967de50

joostlek requested changes Jul 15, 2025

View reviewed changes

home-assistant Bot marked this pull request as draft July 15, 2025 20:34

patrickvorgers added 3 commits August 17, 2025 16:57

Merge branch 'dev' into idrive-e2-backup-provider

679d0b3

Updated configflow to use self.add_suggested_values_to_schema

115611b

Merge branch 'home-assistant:dev' into idrive-e2-backup-provider

be1b46c

patrickvorgers mentioned this pull request Aug 18, 2025

IDrive e2 backup documentation home-assistant/home-assistant.io#40499

Merged

8 tasks

patrickvorgers added 2 commits August 21, 2025 16:37

Rewrite to use pypi idrive_e2_client

66c4cdd

Updated invalid_bucket_name to provide a clear, actionable resolution.

2289667

patrickvorgers marked this pull request as ready for review August 21, 2025 18:56

home-assistant Bot requested a review from joostlek August 21, 2025 18:56

joostlek previously requested changes Sep 12, 2025

View reviewed changes

home-assistant Bot marked this pull request as draft September 12, 2025 09:44

patrickvorgers added 2 commits September 14, 2025 13:44

Solved review comments

9ec334b

Merge branch 'dev' into idrive-e2-backup-provider

d9064c4

patrickvorgers marked this pull request as ready for review September 14, 2025 14:06

patrickvorgers and others added 4 commits February 8, 2026 21:02

Rewrote close test case exception handling

19f7229

Mask SECRET_ACCESS_KEY

6386d43

Reword invalid_credentials to be credential/permission-focused

d470e4f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Check bucket not found on code 404

df6be4d

patrickvorgers marked this pull request as ready for review February 8, 2026 22:12

home-assistant Bot requested a review from zweckj February 8, 2026 22:12

emontnemery closed this Feb 11, 2026

emontnemery reopened this Feb 11, 2026

joostlek reviewed Feb 11, 2026

View reviewed changes

patrickvorgers added 9 commits February 11, 2026 13:04

Removed cm (context manager) variable

3e23942

Patch library where used

06e87c9

Removed aiohttp mock

c2158cb

Created default for list_buckets in mock_client

3a4e625

Use mock_config_entry

9d48d28

Remove usage of async_setup_entry en test

39f151c

Patch where used

b74ab98

Parametrize head_bucket error tests

20f5978

Merge branch 'dev' into idrive-e2-backup-provider

55662ab

patrickvorgers requested a review from joostlek February 11, 2026 17:02

Merge branch 'dev' into idrive-e2-backup-provider

fc701b6

zweckj approved these changes Feb 12, 2026

View reviewed changes

zweckj merged commit d930755 into home-assistant:dev Feb 12, 2026
46 checks passed

This was referenced Feb 13, 2026

S3 backup - Improved buffer handling #162955

Merged

Cloudflare R2 backup - Improved buffer handling #162958

Merged

github-actions Bot locked and limited conversation to collaborators Feb 14, 2026

patrickvorgers deleted the idrive-e2-backup-provider branch March 8, 2026 21:26

		# Go back to the user step if there are errors getting buckets
		# Prefill the access key and secret key fields with the current values

Uh oh!

Conversation

patrickvorgers commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Additional information

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickvorgers Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickvorgers Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickvorgers Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

home-assistant Bot commented Jul 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

patrickvorgers commented May 14, 2025 •

edited

Loading

patrickvorgers Aug 17, 2025 •

edited

Loading

patrickvorgers Aug 17, 2025 •

edited

Loading

patrickvorgers Aug 17, 2025 •

edited

Loading