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

Webhooks for syncing and automation #1649

Closed
wants to merge 1 commit into from

Conversation

subdavis
Copy link

@subdavis subdavis commented May 5, 2020

This is my attempt at a preliminary solution to #573. That's a pretty popular issue, and I think it deserves to be addressed.

See my comment on the issue for more details: #573 (comment)

Adds new config, example:

webhooks:
- url: http://target.url/endpoint
  authorization: "Token iuaorfjaljinfilauhiolkjsnaliufvwqier"
  categories:
  - dns_config
  - dns_rewrite
  - filter

I've opened this issue to start a discussion with the maintainers, but if it makes sense to do something like this, we still need

  • tests
  • documentation
  • UI support (maybe we can push this off for now?)

I've also created https://github.com/subdavis/adguardhome-sync as a proof-of-concept microservice to subscribe to these webhooks and propagate updates. I think the cleverness of that project comes from using TypeScript together with openapi-generator so that future API changes will be detectable at build-time, making it easy to maintain.

EDIT: removed DRAFT status to avoid confusion. Even though this is a work in progress, I'm asking for feedback now.

@subdavis subdavis changed the title Webhooks Webhooks for syncing and automation May 5, 2020
@subdavis subdavis force-pushed the webhooks branch 2 times, most recently from 0196581 to ab31535 Compare May 6, 2020 02:09
@subdavis subdavis marked this pull request as ready for review May 7, 2020 19:37
@ameshkov
Copy link
Member

ameshkov commented May 8, 2020

Hi @subdavis, thank you for the PR, and sorry for the late reply and for messing it up by my refactoring:(

Could you please explain why do you need webhooks and how you're going to use it?

@ameshkov
Copy link
Member

ameshkov commented May 8, 2020

Ah, my bad, I see why. Let me please think about it a little bit.

@subdavis subdavis closed this May 19, 2020
@subdavis
Copy link
Author

I lost interest, sorry.

FWIW, I think this was the correct approach. It doesn't make sense to have users pushing and pulling entire configuration files when all they wanted to do was make 2 instances enable and disable in unison or synchronize block lists. It also doesn't really make sense to me to discard information about the event source: in all observer/observable patterns, event source is the most important piece of data, and removing it would really just result in users relying on other leaky abstractions to make a best guess at what the event source was.

Maybe PiHole, AdGuard Home, Blocky, or something else will eventually support this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants