-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Add unique ID to scripts #70345
Add unique ID to scripts #70345
Conversation
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
@@ -319,6 +319,10 @@ def __init__(self, hass, object_id, cfg, raw_config, blueprint_inputs): | |||
self.description = cfg[CONF_DESCRIPTION] | |||
self.fields = cfg[CONF_FIELDS] | |||
|
|||
# The object ID of scripts need / are unique already | |||
# they cannot be changed from the UI after creating | |||
self._attr_unique_id = object_id |
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.
hmm actually I don't think this will work.
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.
Consider this YAML:
script hello:
example:
...
script:
example:
...
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.
It does work, as its already bit sketchy actually 😮
async def async_validate_config(hass, config):
"""Validate config."""
scripts = {}
for _, p_config in config_per_platform(config, DOMAIN):
for object_id, cfg in p_config.items():
cfg = await _try_async_validate_config_item(hass, object_id, cfg, config)
if cfg is not None:
scripts[object_id] = cfg
Meaning if a script object ID exists twice, it gets overwritten.
Thus never the same one exists twice.
I think it would be nice to throw a warning in the config validation (instead of just silently overwriting). Will do that in a separate PR.
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.
I don't know how to handle this globally but the problem of something in YAML overriding something else exists not only for scripts. I notice this few times that users want to device two entities/devices so they add a section twice and complain why only the last one is working. If we have a way to globally catch this and log a warning it would be better.
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.
Proposed change
Add a unique ID to the scripts entities, using the same method as we do with all
input_*
integrations.The key of a script already has to be unique, thus can be used for scripts as well.
fixes: https://twitter.com/JHerbY2K/status/1516860076316835841
Type of change
Additional information
Checklist
black --fast 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
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: