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

RFC: Call services directly #18720

Merged
merged 11 commits into from
Nov 30, 2018
Merged

RFC: Call services directly #18720

merged 11 commits into from
Nov 30, 2018

Conversation

balloob
Copy link
Member

@balloob balloob commented Nov 26, 2018

Description:

We used to call services via the event bus. This was done to be more event driven. This also allowed back in the day to get multiple Home Assistants working together. This functionality was never fully implemented and has since been deprecated and removed.

This PR aims to simplify the service calling code. We still distinguish between blocking and non-blocking, but now exceptions are able to propagate up. This is especially important for tracking incidents with the unauthorized exception.

This is an RFC. I mainly wanted to see how fast it would be to get done… and it was fairly simple. It's a backwards compatible change.

Breaking change: The service_executed event is no longer being fired when a service is done.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from a team as a code owner November 26, 2018 14:57
@ghost ghost assigned balloob Nov 26, 2018
@ghost ghost added the in progress label Nov 26, 2018
@balloob
Copy link
Member Author

balloob commented Nov 26, 2018

I will not fix the tests until I got some feedback about this idea.

@MartinHjelmare
Copy link
Member

MartinHjelmare commented Nov 26, 2018

What event info are we loosing with this? There's still an event fired after executing the service, right?

@balloob
Copy link
Member Author

balloob commented Nov 26, 2018

So we will still fire an event that we're executing a service and that shares the context with all following changes.

The only event that is missing is "service executed", which always only contained an ID to map it back to the blocked service call.

The thing we're getting is that for blocking calls we get direct feedback that the service doesn't exist or if the data is invalid, which we can now communicate back to the caller. The timeout eventually can also move out of the core and should really be controlled by the caller.

homeassistant/core.py Outdated Show resolved Hide resolved
homeassistant/core.py Show resolved Hide resolved
@pvizeli pvizeli merged commit df21dd2 into dev Nov 30, 2018
@ghost ghost removed the in progress label Nov 30, 2018
@delete-merged-branch delete-merged-branch bot deleted the service-direct branch November 30, 2018 20:28
@balloob balloob mentioned this pull request Dec 12, 2018
DubhAd added a commit to home-assistant/home-assistant.io that referenced this pull request Dec 19, 2018
Apparently this was missed with the 0.84 updates 

home-assistant/core#18720
fabaff pushed a commit to home-assistant/home-assistant.io that referenced this pull request Dec 19, 2018
Apparently this was missed with the 0.84 updates 

home-assistant/core#18720
alengwenus pushed a commit to alengwenus/home-assistant.io that referenced this pull request Feb 24, 2019
Apparently this was missed with the 0.84 updates 

home-assistant/core#18720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants