nsot icon indicating copy to clipboard operation
nsot copied to clipboard

REST Hooks

Open coxley opened this issue 10 years ago • 7 comments

@jathanism and I talked in #nsot and dropbox/pynsot#55 about this.

This is really important to have for me and brings Trigger + source of truth integration so much closer (better, at least).

Endless possibilities here, but for a Trigger use-case keeping a NetDevices loader up to date would be easy and fast. Could let Trigger do what it does best at loading via JSON, YAML, Mongo, SQLite, etc and just run a small service to catch the updates from NSoT.

References:

  • http://resthooks.org/
  • https://github.com/zapier/django-rest-hooks

Need to decide on what events we want to create. Obvious ones (I think) would be:

  • Site
  • Attribute
  • Network
  • Device

coxley avatar Sep 11 '15 06:09 coxley

Ok, deep linking this to the Trigger issue (https://github.com/trigger/trigger/issues/231) adds the missing context to the hooks suggestion.

jathanism avatar Sep 15 '15 16:09 jathanism

@jathanism: Woo!... kind of.

I have hooks "working". Meaning, everything that should work works except for the fact that this module forces you to have user attribute per model meaning it wants you to subscribe to only things you own, per user. For a single source of truth, this is not proper behavior! I could understanding subscribing based on $criteria, but not who submitted it.

https://github.com/zapier/django-rest-hooks/issues/15

I did change the schema at first for Site, Network and Device before going "Wait, what am I doing it doesn't store this!":

    user = models.ForeignKey(
        settings.AUTH_USER_MODEL, related_name='sites', db_index=True
    )

It's late, but let's chat about this tomorrow.

coxley avatar Sep 16 '15 07:09 coxley

Also,

https://github.com/PressLabs/django-rest-hooks-ng

Without looking too much into it, this appears to be more what we're after with global hooks. It's a fork of django-rest-hooks, but apparently it changes the schema away from the main project a bit? Either way we're not set up using the original project in the first place so wouldn't matter.

coxley avatar Sep 16 '15 07:09 coxley

@coxley Am I mistaken or isn't the existing use-case for appending a + to an event name designed to solve this? e.g. from the README:

    # ... If you want a Hook to
    # be triggered for all users, add '+' to built-in Hooks
    # or pass user_override=False for custom_hook events

Either way, since this use-case is one you're running with, I'll leave it up to you to decide.

jathanism avatar Sep 16 '15 15:09 jathanism

@jathanism That's what I assumed which is why if you look at [https://github.com/dropbox/nsot/compare/master...coxley:hooks](my fork) you'll see that's what I did.

The issue I linked is pretty fresh and that line in the README was added a month or two before the issue was raised, where the maintainer confirmed that it wasn't easy to use for that case.

But yeah, I'll run with the other project and hope it's about the same code. Still getting used to Django but starting to realize the built-in admin stuff comes in handy. :+1

coxley avatar Sep 16 '15 16:09 coxley

@jathanism Just got around to checking the other project and it just allows you to specifiy global_hook as a bool in the payload when creating a hook.

In other words, to support this we need to have Site, Device, and Network have a user prop like Change does. I imagine we can work this out completely server-side; would be lame to provide user details in HTTP headers and then have to do so again in payload.

Will reference the source some more, specifically how Change is populated.

coxley avatar Sep 17 '15 05:09 coxley

Just an update here:

Following IRC conversations we came to the realization that this would be way more elegant if implemented solely on the Change model considering everything ends up there anyway and enables us to define just two extra model methods.

Still in testing currently.

coxley avatar Sep 17 '15 20:09 coxley