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

REST Hooks #83

Open
coxley opened this issue Sep 11, 2015 · 7 comments
Open

REST Hooks #83

coxley opened this issue Sep 11, 2015 · 7 comments

Comments

@coxley
Copy link
Contributor

coxley commented Sep 11, 2015

@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:

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

  • Site
  • Attribute
  • Network
  • Device
@jathanism
Copy link
Contributor

Ok, deep linking this to the Trigger issue (trigger/trigger#231) adds the missing context to the hooks suggestion.

@coxley
Copy link
Contributor Author

coxley commented Sep 16, 2015

@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.

zapier/django-rest-hooks#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
Copy link
Contributor Author

coxley commented Sep 16, 2015

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.

@jathanism
Copy link
Contributor

@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.

@coxley
Copy link
Contributor Author

coxley commented Sep 16, 2015

@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
Copy link
Contributor Author

coxley commented Sep 17, 2015

@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
Copy link
Contributor Author

coxley commented Sep 17, 2015

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.

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

No branches or pull requests

2 participants