core
core copied to clipboard
Scrape move yaml config to integration key
Breaking change
Loading Scrape via platform key has been deprecated. Please move your config directly onto the scrape key.
scrape:
- resource: ...
Proposed change
Makes the following changes
Reference: https://github.com/home-assistant/core/pull/70476#issuecomment-1172509117
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [x] New feature (which adds functionality to an existing integration)
- [ ] 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: https://github.com/home-assistant/home-assistant.io/pull/23262
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:
- [x] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [x] 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.
The integration reached or maintains the following Integration Quality Scale:
- [ ] No score or internal
- [ ] 🥈 Silver
- [ ] 🥇 Gold
- [ ] 🏆 Platinum
To help with the load of incoming pull requests:
- [x] I have reviewed two other open pull requests in this repository.
Hey there @fabaff, mind taking a look at this pull request as it has been labeled with an integration (scrape) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)
~~Documentation needs updating. Should merge https://github.com/home-assistant/home-assistant.io/pull/23253 first and then make new.~~ Completed
and then make new. Merged! 👍
Not sure what codecov is doing. It's compaining about all empty lines and docstrings
Codecov is doing something wrong. Tests are 100% coverage
---------- coverage: platform linux, python 3.9.13-final-0 -----------
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------
homeassistant/components/scrape/__init__.py 50 0 100%
homeassistant/components/scrape/config_flow.py 24 0 100%
homeassistant/components/scrape/const.py 8 0 100%
homeassistant/components/scrape/sensor.py 116 0 100%
------------------------------------------------------------------------------
TOTAL 198 0 100%
Hi @gjohansson-ST,
I was looking at the REST sensor earlier today (see #80543, #80544, #80546) and that is now using a DataCoordinator for sensor and binary_sensor platforms.
Would it make sense to implement that right away as part of the migration, so we don't have a new breaking change so soon after the migration to the integration key?
Also, I am confused around the Config Flow tweaks that are in this PR. Surely those should be in a separate PR...
@epenet Yes. Let's do a data coordinator right away. Don't think the config flow is with tweaks, it's pretty straight forward, but you're right. I will remove that and we do that in a separate PR.
I think #81130 is the last preliminary PR I have in mind for scrape. After that we can focus again on the "integration YAML"
For the "integration YAML", I really think that we need to implement a two-level configuration:
scrape:
- resource: https://example.com
authentication: basic
username: username
password: password
sensors:
- name: sensor 1
select: td
index: 1
unit_of_measurement: °C
- name: sensor 2
select: td
index: 2
unit_of_measurement: °C
The coordinator would be created and tested inside __init__.py, and then included in the data passed to sensor.py
Then in sensor.py, we can create the coordinator for old config, and use the passed coordinator for new config.
The coordinator would be created and tested inside
__init__.py, and then included in the data passed tosensor.pyThen insensor.py, we can create thecoordinatorfor old config, and use the passed coordinator for new config.
Worked on getting this on track yesterday but was a chunk to go through so if you have a look now when I published it should be pretty aligned with previous PR's.
Still requires cleanups and adapting the sensor tests which I will do next.
Just a thought, could you add a comment here with the coverage result, and a small note about the coverage failure?
Coverage 100% besides config_flow.py which is a left-over from previous PR where config flow was implemented and later reverted.
Config flow intended to be implemented in follow-up PR so file therefore left as is.
---------- coverage: platform linux, python 3.9.15-final-0 -----------
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------
homeassistant/components/scrape/__init__.py 33 0 100%
homeassistant/components/scrape/config_flow.py 24 24 0% 2-121
homeassistant/components/scrape/const.py 9 0 100%
homeassistant/components/scrape/coordinator.py 17 0 100%
homeassistant/components/scrape/sensor.py 83 0 100%
------------------------------------------------------------------------------
TOTAL 166 24 86%
Looks good to me. ~But I've probably been involved too closely so I'd like a second opinion.~
I think we can merge this and we can address comments in a follow-up PR.