-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
Initial import for HassIO #6935
Conversation
homeassistant/components/hassio.py
Outdated
DOMAIN, SERVICE_ADDON_UNINSTALL, | ||
descriptions[DOMAIN][SERVICE_ADDON_UNINSTALL], | ||
schema=SCHEMA_SERVICE_ADDONS) | ||
hass.services.async_register( |
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 wonder if services is the right way to expose management of the system. The alternative is an API that we call from a specialized frontend panel. The nice thing about API is that we can have return values and know if a service was a success etc.
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.
We have no access from outside to supervisor. I think that is the best thing for security. So we need passtrough this stuff over hass.
I design the thing in the way that we call /xy/info with config pannel to load data and send it to frontend. So we need only call this getter (like the code from hassbian config) to rebuild infos after service call.
homeassistant/components/hassio.py
Outdated
@asyncio.coroutine | ||
def async_service_handler(service): | ||
"""Handle HassIO service calls.""" | ||
if service.service == SERVICE_HOST_UPDATE: |
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 suggest you create a dict that maps services to commands, payloads and schemas and lookup that here. See cover component for example. It will use a lot fewer lines of code.
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.
done
homeassistant/components/hassio.py
Outdated
yield from hassio.send_command("/addons/{}/stop".format(addon)) | ||
elif service.service == SERVICE_ADDON_UPDATE: | ||
yield from hassio.send_command( | ||
"/addons/{}/update".format(addon), payload=version) |
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.
If you make SERVICE_MAP
nested you could include a key-pair for the command string and another key-pair for the payload. Then have two cases at lookup-time, ie here, one where addon
is None
which just passes the command string to the send_command
method and another case where addon
is not None
which uses string formatting on the command string before passing it along.
You would not need this long if - else trail.
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 think we can do that later. for the moment it help for more readable of code and maby we change some things in next version. That is also the reason of v1 on name :)
Ready to merge 💃 |
homeassistant/components/hassio.py
Outdated
try: | ||
with async_timeout.timeout(TIMEOUT, loop=self.loop): | ||
request = yield from self.websession.get( | ||
"http://{}{}".format(self._ip, cmd), |
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.
What if self._ip
is None, this will return an error?
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.
Resolving IP should also happen inside setup
, as it's a Home Assistant concern where it comes from.
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.
We can do that. But without IP it will not load the component
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.
done
@asyncio.coroutine | ||
def post(self, request, addon): | ||
"""Set options on host.""" | ||
data = yield from request.json() |
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.
We should have voluptuous schemas for data. That way any vulnerability in hassio won't be exposed.
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 think we make only a passthrought. HassIO will give a error back to frontend if a option will not support it.
@asyncio.coroutine | ||
def get(self, request): | ||
"""Get base data.""" | ||
data = yield from self.hassio.send_command(self._url_info) |
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.
Should we wrap these in async_timeout
?
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.
We do that on HassIO object
"""Match a request against pre-registered requests.""" | ||
data = data or json |
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.
We should make sure the json is converted to a string and stored in data. That way we can match the string data = "{}"
against json = {}
(as the requests are the same)
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.
That make it bad for tests... I need revert it on every test case. That will be nonsense
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.
At the end we use that only inside tests, If possible that we have that test friendly?
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.
oh good point, I thought it was used for matching. This is ok.
homeassistant/components/hassio.py
Outdated
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
TIMEOUT = 900 |
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.
Why is this timeout so high?
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.
A docker image can have a long time to pull
homeassistant/components/hassio.py
Outdated
@property | ||
def connected(self): | ||
"""Return True if it connected to HassIO supervisor.""" | ||
return self._ip is not None |
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.
Should there be a no-op API command so that we can connect to HASSIO and verify that we can make a connection? We do something similar for our normal API https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/remote.py#L81-L86
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.
Ok
* Initial import for HassIO * Cleanup api code for views * First unittest for view * Add test for edit view * Finish unittest * fix addons test * cleanup service.yaml * Address first round with ping command * handle timeout dynamic * fix lint
Cherry-picked into the 0.42 branch |
Description:
The next generation of IoT device is comming, HassIO. It provide image for most of avilable IoT hardware and make this device maintenance free and easy to use. It is the first private cloud home solution for HomeAssistant and abstract IoT to a new Level with virtualisation. On this place a big thanks to Resin Team.
HassIO become a full UI Integration into hass and make Hardware and HomeAssistant to one peace. At the moment it is not for production use!
If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass