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

Add webhook + IFTTT example #16817

Merged
merged 10 commits into from Sep 30, 2018

Conversation

@balloob
Member

balloob commented Sep 24, 2018

Description:

This adds a new webhook component as proposed in home-assistant/architecture#80

To test it, I have updated the IFTTT component with a config flow to register a webhook.

screen shot 2018-09-24 at 4 54 31 pm

If we like architecture, I can add tests. Tests added

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

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

This comment has been minimized.

Member

balloob commented Sep 24, 2018

One thing that I don't like about this approach is that we show the config info before the user confirms that they want to set it up. Would be better once they hit submit, however, we don't currently support showing a message when a config entry has been created.

@balloob

This comment has been minimized.

Member

balloob commented Sep 24, 2018

We should also abort if the host is a local IP address

@balloob

This comment has been minimized.

Member

balloob commented Sep 24, 2018

image

@bind_hass
def async_register(hass, webhook_id, handler):
"""Register a webhook."""
handlers = hass.data.setdefault(webhook_id, {})

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 24, 2018

Member

Why do we need a dict to store the handler when each webhook id will only get one handler?

This comment has been minimized.

@balloob

balloob Sep 24, 2018

Member

ugh that is a mistake, that should have been DOMAIN.

This comment has been minimized.

@balloob

balloob Sep 24, 2018

Member

(still need to write tests)

})
assert result['type'] == data_entry_flow.RESULT_TYPE_FORM, result
result = await hass.config_entries.flow.async_configure(result['flow_id'], {})

This comment has been minimized.

@houndci-bot

houndci-bot Sep 25, 2018

line too long (82 > 79 characters)

Show resolved Hide resolved tests/components/ifttt/test_init.py
@balloob

This comment has been minimized.

Member

balloob commented Sep 25, 2018

Tests added, this is ready for review.

@balloob

This comment has been minimized.

Member

balloob commented Sep 28, 2018

CC @dgomes I think that the webhook component is what the push camera could use too.

@dgomes

This comment has been minimized.

Member

dgomes commented Sep 28, 2018

Sure thing, I'm currently a bit overwhelmed at work but will issue a PR with this ASAP

@balloob balloob merged commit f5632a5 into dev Sep 30, 2018

6 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 93.54%
Details

@wafflebot wafflebot bot removed the in progress label Sep 30, 2018

@balloob balloob deleted the webhook branch Sep 30, 2018

@kirichkov kirichkov referenced this pull request Oct 3, 2018

Merged

Webhook component - pass headers to webhook handler #17091

2 of 2 tasks complete

@rohankapoorcom rohankapoorcom referenced this pull request Oct 15, 2018

Merged

Migrate Mailgun to use the webhook component #17464

8 of 8 tasks complete

@dgomes dgomes referenced this pull request Nov 9, 2018

Closed

Camera push webhook #18346

9 of 9 tasks complete

@dgomes dgomes referenced this pull request Nov 11, 2018

Open

replace token in camera.push with webhook #18380

9 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment