Skip to content

Backend changes for customize config panel.#9134

Merged
balloob merged 5 commits into
home-assistant:devfrom
andrey-git:c2
Aug 26, 2017
Merged

Backend changes for customize config panel.#9134
balloob merged 5 commits into
home-assistant:devfrom
andrey-git:c2

Conversation

@andrey-git
Copy link
Copy Markdown
Contributor

@andrey-git andrey-git commented Aug 25, 2017

Description:

Backend changes for customize config panel.

This is a potentially breaking change as someone using the new config UI can overwrite their manual customize.yaml and lose their comments.

Example entry for configuration.yaml (if applicable):

homeassistant:
  customize: !include customize.yaml

config:

Checklist:

Tests are not written yet.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link
Copy Markdown

@andrey-git, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob and @fabaff to be potential reviewers.

@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 26, 2017

Can you update homeassistant/config.py to automatically create the customize.yaml file. Also needs to update the test for that file to clean it up after a test run.

"""Set up the Customize config API."""
@asyncio.coroutine
def hook(hass):
"""Hook to call after updating cutomization file."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"customization"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this function

def async_setup(hass):
"""Set up the Customize config API."""
@asyncio.coroutine
def hook(hass):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the async version and import it from homeassistant.components (it already has the sync version of this method)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

class CustomizeConfigView(EditKeyBasedConfigView):
"""Configure a list of entries."""

def _get_value(self, data, config_key):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the method signatures for get and write value to include hass, that way you don't have to do declare the class inline.

hass.states.async_set(config_key, state.state, state_attributes)

hass.http.register_view(CustomizeConfigView(
'customize', 'config', CONFIG_PATH, cv.entity_id, dict,
Copy link
Copy Markdown
Member

@balloob balloob Aug 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if we should change the first two parameters to be 'homeassistant', 'customize'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first is not really a "domain", so I think the current values are OK.

@balloob balloob merged commit c73338b into home-assistant:dev Aug 26, 2017
@balloob balloob mentioned this pull request Sep 7, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Dec 11, 2017
@andrey-git andrey-git deleted the c2 branch May 10, 2018 18:14
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.

5 participants