Skip to content

Add automation options for parallel action runs#31740

Closed
pnbruckner wants to merge 1 commit intohome-assistant:devfrom
pnbruckner:automation-retrigger
Closed

Add automation options for parallel action runs#31740
pnbruckner wants to merge 1 commit intohome-assistant:devfrom
pnbruckner:automation-retrigger

Conversation

@pnbruckner
Copy link
Copy Markdown
Contributor

Breaking change

Previously if the action section of an automation contained a delay or wait_template step, and a trigger fires while the automation is still in one of these steps from a previous trigger event, the step would be aborted and the action would continue with the next step. This was apparently unintentional behavior and many people have been surprised by it, and have had to work around it.

This change fixes that behavior by allowing the user to specify what should happen when the automation is triggered while it is still running its actions from a previous trigger event. The default behavior in the scenario described above will be to stop the previous run and start the action sequence over from the beginning. Although this is probably what most people would have expected, it can break existing automations that depend on the current odd behavior.

Proposed change

Add a new, optional configuration parameter for automations, namely parallel_action, which will control what happens when the automation is triggered while the action sequence is still running from a previous trigger event. There are four options:

  • allow -- The action sequence will be run in parallel with the previous run. This is the default if the action sequence does not contain a delay or wait_template.
  • error -- The trigger will cause an ERROR.
  • restart -- The previous run will be cancelled before a new run is started. This is the default if the action sequence does contain delay or wait_template.
  • skip -- The trigger will be ignored and the previous run will continue unaltered.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • 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

Example entry for configuration.yaml:

# Example configuration.yaml
automation:
- trigger:
    platform: state
    entity_id: binary_sensor.motion
    to: 'on'
  action:
  - service: light.turn_on
    entity_id: light.porch
  - delay: "00:05:00"
  - service: light.turn_off
    entity_id: light.porch

Additional information

Detailed list of changes

  • Add optional automation config parameter parallel_action. Options and default are defined in Script helper class.
  • Move async_log_exception functionality entirely inside Script class.
  • Automation uses Script's last_triggered attribute.
  • Enhance Script class to properly support simultaneous parallel runs regardless of whether the script sequence contains delays or wait_templates.
  • Four options are available for what happens if script is run while it is already running: allow, error, restart & skip.
  • All run context is moved into a new helper class _ScriptRun.
  • Only update last_triggered when Script first run (i.e., not when resumed after delay or wait_template.)
  • Fix bug where wait_template listener is not canceled when timeout expires.
  • Update tests.

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

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 running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@probot-home-assistant
Copy link
Copy Markdown

Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with a integration (script) you are listed as a codeowner for? Thanks!

@probot-home-assistant
Copy link
Copy Markdown

Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with a integration (automation) you are listed as a codeowner for? Thanks!

@pnbruckner
Copy link
Copy Markdown
Contributor Author

pnbruckner commented Feb 11, 2020

Since helper.script was reorganized it can be a bit difficult to review here on github. Also, I've been thinking that I should have put the new _ScriptRun class inside the Script class. And if I do it right it should make the differences much easier to read & understand. I'll start working on that next and push a new commit if it does indeed help.

…n runs

- Add optional automation config parameter parallel_action. Options and
  default are defined in Script helper class.
- Move async_log_exception functionality entirely inside Script class.
- Automation uses Script's last_triggered attribute.
- Enhance Script class to properly support simultaneous parallel runs
  regardless of whether the script sequence contains delays or
  wait_templates.
- Four options are available for what happens if script is run while it
  is already running: allow, error, restart & skip.
- All run context is moved into a new helper class _ScriptRun.
- Only update last_triggered when Script first run (i.e., not when resumed after
  delay or wait_template.)
- Fix bug where wait_template listener is not canceled when timeout expires.
- Update tests.
@pnbruckner
Copy link
Copy Markdown
Contributor Author

Ok force pushed a new commit. This one should be easier to review. Still, you'll probably be better off using a better diff tool (for helpers/script.py) than github. lol

@codecov

This comment has been minimized.

@pnbruckner
Copy link
Copy Markdown
Contributor Author

Working on adding more tests for helpers/script.py.

Comment thread homeassistant/components/automation/__init__.py
Comment thread homeassistant/components/script/__init__.py
Comment thread homeassistant/helpers/script.py
Comment thread homeassistant/helpers/script.py
Comment thread homeassistant/helpers/script.py
Comment thread homeassistant/helpers/script.py
Comment thread homeassistant/helpers/script.py
Comment thread homeassistant/helpers/script.py
@pnbruckner
Copy link
Copy Markdown
Contributor Author

Closing for now. Will submit a new PR with the first, smaller set of changes to helpers/script.py & its tests hopefully before too long.

@pnbruckner pnbruckner closed this Feb 13, 2020
@lock lock Bot locked and limited conversation to collaborators Feb 14, 2020
@balloob
Copy link
Copy Markdown
Member

balloob commented Feb 16, 2020

When you open any of the new PRs, please request my review and I will expedite them.

@pnbruckner
Copy link
Copy Markdown
Contributor Author

pnbruckner commented Feb 17, 2020

Thanks, will do. I haven't forgotten or given up. Still working on it. I hope to have the first one in the next few days. I've already written a few new tests to make sure I get the legacy mode stuff right.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automation action delay not working

4 participants