core
core copied to clipboard
Adjust REST schema validation
Proposed change
Linked to #81551
In 2022.10, passing an empty configuration to rest resulted in a successful setup, with a failing coordinator and no sensors.
rest:
In 2022.11, code was adjusted on the coordinator to ensure we had a valid resource before creating the dataclass, which now raises a HomeAssistantError:
raise HomeAssistantError("Resource not set for RestData")
The side effect of this change is that it highlited invalid configurations in the real world.
Should it be adjusted? I am not sure. I am proposing this PR, but I am open to suggestion to handle it better. Or maybe we don't need to handle it...
Type of change
- [ ] Dependency upgrade
- [x] 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: https://github.com/home-assistant/core/issues/81551
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] The code has been formatted using Black (
black --fast homeassistant tests) - [x] 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 runningpython3 -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.
- [ ] Untested files have been added to
.coveragerc.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
A hard fail is better here since it can be caught by a configuration check and the error doesn't get hidden away leaving the user to guess what went wrong. It just isn't so much fun on an upgrade.
I'd go back an relabel the existing case a breaking change and update the release notes so it makes the one time pain a bit easier to resolve.
Why would an empty test block be a fail? It should just be a loaded component with no configured sensors.
Why would an empty test block be a fail? It should just be a loaded component with no configured sensors.
If the schema validation returned an empty list that would work. But I'm not sure how to adjust it accordingly.
I think it's a bug that it can't cope with an empty included file. I also have
group: !include groups.yaml
And my "groups.yaml" file is empty, but there is no error. So I think it needs to be able to cope with empty file without an error or warning.
I think the latest commit 7bd7b2f resolve this issue.
If an empty dict is passed to the schema validation, it now gets converted to an empty list.
Without this patch, it was being converted to a list with a single dict with injected defaults:
[{'verify_ssl': True, 'timeout': 10, 'method': 'GET'}],