Add pingdom.com as an integration about zulip HOT 11 CLOSED

Opened #70 with some notes on how to add an integration if anyone's interested.

from zulip.

@timabbott
Here we should use webhooks instead of API .
API allows for the same actions like web interface but configuration the API (even if we make it easier) requires from user some technical skills. And its additional not needed maintenance of course

So I prefer using webhooks.
User did some checker in pingdom.com then add a hook that we handle.
This way does not require from the user doing the same configuration twice. And, what is the most important, user can use graphical interface of pingdom.

Small architecture question:
webhooks.py has more than 1000 lines. Quite a lot ;-)
What do you think about quick refactor?

webhooks:
    __init__.py
    common.py
    beanstalk.py
    github.py
    . (another)
    pingdom.py
    . (another)
    travis.py

In common.py we can have abstract class like

class WebhookHandlerAbstract(object):
    def handle():
     ....

In init we can have just simple functions like:

def api_benstal_webhook(...):
     BenstalkWebhookHandler().handle()

@timabbott Quick answer is welcome due to just 14 days trial of pingdom ;-)

from zulip.

Using webhooks here sounds good to me.

I agree that we will want to split webhooks.py into one-file-per-hook. If you choose to work on that, please write your changes in a way that's easy to review and carefully follow the Zulip commit guidelines http://zulip.readthedocs.org/en/latest/code-style.html#version-control

So for that refactor, I'd recommend first a commit moving webhooks.py to webhook/init.py, and then individual commits moving individual integrations out of the common file.

from zulip.

Also I would avoid combining the change to split out the files with a change to add a WebhookHandlerAbstract construct; it usually works best to avoid combining two totally different refactors into the same commit. (does this make sense?)

Also, at present there's little in common between the various webhooks, so I don't think there's a reason to create a WebhookHandlerAbstract class anyway.

from zulip.

WebhookHandlerAbstract is not good name/type - it's just an interface.

from zulip.

#513
Please take a look @timabbott

from zulip.

TomaszKolek#1

@timabbott
It's on my fork because I'm working on refactored webhooks version.
Please look if we've not missed our concepts.

from zulip.

Cool! That approach looks pretty good.

from zulip.

#537 - created, ready to review.

from zulip.

Merged in 44ed9da, thanks @TomaszKolek for building this!

from zulip.

FYI @zack-ahdach.

from zulip.