-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
MVP implementation of Backup sync agents #126122
base: dev
Are you sure you want to change the base?
Conversation
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
You only need to include what you want to expose in __all__
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.
Looks good! Tests are missing. Some small comments and questions.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
One test is failing. |
@@ -313,8 +314,11 @@ async def test_agents_download( | |||
} | |||
) | |||
with patch.object(BackupSyncAgentTest, "async_download_backup") as download_mock: | |||
assert await client.receive_json() | |||
assert snapshot == download_mock.call_args | |||
assert snapshot == await client.receive_json() |
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 put the expected value, in this case the snapshot, on the right side of the assertion.
When adding new websocket commands we normally wait with merging the core PR until there's an approved frontend PR. If the API is already settled with the frontend that may not be needed. |
Mypy is failing. |
Proposed change
Ref: home-assistant/architecture#1134
Based on the proposal above, this PR implements the MVP of backup sync agents.
As of now, this will only be invoked by the core backup integration.
Changes to allow these to be triggered in a supervisor environment will come in a different PR.
So this adds:
async_get_backup_sync_agents
function in the backup platform (as shown with thekitchen_sink
integration)Note that this PR does not yet add any tests, nor is related developer and user docs updated, this is on purpose as I expect the contents of this PR will change.
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: