Remove auto target all entity when using a blank service call#25715
Remove auto target all entity when using a blank service call#25715rbflurry wants to merge 4 commits into
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #25715 +/- ##
=========================================
- Coverage 94.01% 92.82% -1.2%
=========================================
Files 541 536 -5
Lines 41869 41057 -812
=========================================
- Hits 39365 38110 -1255
- Misses 2504 2947 +443
Continue to review full report at Codecov.
|
| ENTITY_MATCH_ALL, | ||
| ) | ||
|
|
||
| if data_ent_id in (ENTITY_MATCH_ALL): |
There was a problem hiding this comment.
this is a bug. you now check if data_ent_id is a valid subscring of ENTITY_MATCH_ALL
| if data_ent_id in (ENTITY_MATCH_ALL): | |
| if data_ent_id == ENTITY_MATCH_ALL: |
balloob
left a comment
There was a problem hiding this comment.
Looks good. Can be merged when you address formatting and probably the tests too to work with this.
amelchio
left a comment
There was a problem hiding this comment.
This seems too simple. Instead, we should remove the warnings completely and use vol.Required(ATTR_ENTITY_ID) in affected schemas.
| ) | ||
| target_all_entities = True | ||
| 'Not passing an entity ID to a service to target all ' | ||
| 'entities is not supported.', service_name, ENTITY_MATCH_ALL) |
There was a problem hiding this comment.
The comment above this logging statement mentions that the service_name function parameter was only used for this warning and should be removed along with it.
There was a problem hiding this comment.
Do we still want a warning? or should I drop the warning all together.
|
@rbflurry are you planning to continue here and address the last comment? |
|
This PR is now running stale for 3 months and therefore, going to close it. Your contribution would be very much appreciated 👍 |
Breaking Change:
Removes the functionality to auto target all entity when using a blank service call. This has been deprecated since December 2018 and version 0.85
Description:
Related issue (if applicable): fixes #
Relates to: #19006
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest.requirements_all.txtby runningpython3 -m script.gen_requirements_all..coveragerc.If the code does not interact with devices: