Skip to content
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

Fix rest_command when server is unreachable #26948

Merged
merged 6 commits into from Sep 30, 2019

Conversation

@sebasje
Copy link
Contributor

commented Sep 27, 2019

When a server doesn't exist, the connection fails immediately, rather
than waiting for a timeout. This means that the async handler is never
reached, and the request variable never filled, yet it's used in the
client error exception handler, so this one bugs out.

By using the command_config, we avoid using the potentially unassigned
request variable, avoiding this problem.

This patch makes scripts work that have a rest_command in them which
fails due to a server being offline.

Description:

I'm using a rest_command to switch off one specific lamp in the middle of a "go to bed" script. When that lamp is offline, the rest_command fails and the remainder of my script never executes (some lights stay on).
The rest_command handler only takes timeouts into account, but not a server being entirely unreachable, apparently. In this case, the request variable that is used only to log the client url never gets assigned, leading to an execution error. The url is also available from the command_config, so if I just use that in the exception handler to print the client url, the remainder of hte scripts finishes fine (and all my lights turn off, and I can sleep peacefully).

Example entry for configuration.yaml (if applicable):

rest_command:
  livinglamp_off:
    url: http://192.168.1.211/effect?effect=off

in scripts.yaml:

ttest:
  alias: TTest
  sequence:
  - service: light.turn_on
    data:
      entity_id: light.living_room
  - data: {}
    service: rest_command.livinglamp_off
  - delay:
      seconds: 5
  - service: light.turn_off
    data:
      entity_id: light.living_room

When a server doesn't exist, the connection fails immediately, rather
than waiting for a timeout. This means that the async handler is never
reached, and the request variable never filled, yet it's used in the
client error exception handler, so this one bugs out.

By using the command_config, we avoid using the potentially unassigned
request variable, avoiding this problem.

This patch makes scripts work that have a rest_command in them which
fails due to a server being offline.
instead of printing the template object
@sebasje

This comment has been minimized.

Copy link
Contributor Author

commented Sep 27, 2019

Could someone please point me to how this should be formatted? For all I can't tell, it's consistent with other usages, and I can't find docs that tell me how to do it otherwise.

Also, I've fixed the cla-error by adding my other email address to githut, yet the label is still assigned.

I guess at this point I need human interaction. :) @homeassistant

@frenck

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

@sebasje CLA looks good now 👍

About formatting, we use a tool called Black to format our code. Run the following command:

black --fast homeassistant from the root of the project, and it will take care of properly formatting your files.

The nice thing about this, it that you as a developer don't have to worry about formatting.

sebasje added 2 commits Sep 27, 2019
@sebasje

This comment has been minimized.

Copy link
Contributor Author

commented Sep 27, 2019

@frenck Ah, thanks, I didn't know black. Fixed the formatting now. :)

Dev automation moved this from Needs review to Review in progress Sep 27, 2019
@home-assistant home-assistant deleted a comment from homeassistant Sep 27, 2019
@MartinHjelmare MartinHjelmare changed the title fix rest_command when server is unreachable Fix rest_command when server is unreachable Sep 27, 2019
@sebasje sebasje requested a review from pvizeli Sep 27, 2019
Dev automation moved this from Review in progress to Reviewer approved Sep 30, 2019
@pvizeli pvizeli merged commit c527e0f into home-assistant:dev Sep 30, 2019
11 checks passed
11 checks passed
CI Build #20190927.44 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing fc700c7...85feca1
Details
codecov/project 94.19% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Sep 30, 2019
@lock lock bot locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
4 participants
You can’t perform that action at this time.