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

Model change events subscription #25

Closed
ramukima opened this issue Aug 14, 2016 · 21 comments
Closed

Model change events subscription #25

ramukima opened this issue Aug 14, 2016 · 21 comments
Assignees
Milestone

Comments

@ramukima
Copy link
Contributor

ramukima commented Aug 14, 2016

Few questions -

  • Does an emitted event indicate what exactly what action was performed against the model e.g. CREATE/UPDATE/DELETE ?
  • Do I have an option to subscribe to only part of the model changes and not the complete model itself. e.g. I am interested in change events for a subscriber model, but only for its "id" field change. If the event subscription API allows to specify an xpath filter to be specified for this kind of "model change interest".
  • Do the event listeners get to know the "delta changes" that happens to a model or just the latest model i.e. prop.content ? I see that the delta changes notion is available in some form for PUT requests.
@sekur sekur self-assigned this Aug 15, 2016
@sekur
Copy link
Collaborator

sekur commented Aug 15, 2016

Eventually, we will support all of the points you raised above. 👍

Right now, it's a bit of a broadcast with a singular update event when any value change occurs inside the Model. Next on the list is a change event when the schema for the Model itself.

The create/delete events likely only applies for list statements and we can either support those as an additional event or in place of the update event for the list property in question. Which one do you think is more appropriate?

As to XPATH expression, I'm trying to come up with the best expression to capture listening for a sub-element of interest. It would be simpler if the XPATH was a way to subscribe to any event from the matching node(s), but probably a bit more involved if we need to somehow express one-or-more specific type of event from the matching node(s).

@ramukima
Copy link
Contributor Author

ramukima commented Aug 15, 2016

Create/Delete events could be applied to container/list entries. I think "update" can be considered a general bucket for pushing events when -

  • A list entry is created
  • A container is instantiated
  • An attribute value (leafy attribute) is SET/CHANGED/UNSET (deleted)

When a list is updated (a new entry for example or change to attributes of existing list entry), I would expect multiple events. i.e. One for update on the list, and others for set on attributes of the list entry attributes. Hope this makes sense.

I agree, its a little tricky to come up with the XPATH expression filters for selective event subscription. However, I meant YPATH instead of XPATH if you will. YPATH is an expression pointing to the model schema while XPATH may point to instance data. I prefer being able to specify YPATH for event subscription filters. For example, if I subscriber to "/subscriber/name" YPATH where 'subscriber' is a list and 'name' is a leaf attribute, I expect events raised when "name" is changed (set/update/delete) for any instance (existing or new) abiding the subscriber data model. Does this make sense ? If not, may be NETCONF specification for notifications may provide hints on what to allow.

sekur added a commit that referenced this issue Aug 16, 2016
@sekur
Copy link
Collaborator

sekur commented Aug 16, 2016

@ramukima - the current update event captures the general bucket for any changes as you've noted.

I've added additional create/delete events to fire when list has elements added/removed. The event fires with (prop, items...) where items are one or more elements added/removed.

The case where one of the attributes for a given list item is updated, only update event is triggered for that attribute in question. However, since the way event system works is via propagation up the tree, you would be able to watch for an update event directly on the list element and also get the event - although the prop argument will be the changed attribute itself and not the list element you're listening on.

Yes, I've also noted a need to have some internal separation between pure XPATH and the new pseudo YPATH construct. It's not entirely clean right now but it will behave in the way you describe. BTW, is YPATH an actual specification or just something we're making up here?

sekur added a commit that referenced this issue Aug 16, 2016
expression as discussed on #25.

cleanup some internal Model schema associations and adjust
Model.access accordingly.
@sekur
Copy link
Collaborator

sekur commented Aug 16, 2016

@ramukima - with the latest update, you can now attach to a specific XPATH filter on a given Model's event:

model = (yang 'list foo { container bar { leaf a; leaf b; } }') foo: [ bar: { a: 'hi', b: 'there' } ]
model.on 'update', '/foo/bar/a', -> console.log "property 'a' updated"
model.foo[0].bar.a = 'bye' # fires the above event callback handler

Hopefully this gives you the granular event subscription you're looking for.

@ramukima
Copy link
Contributor Author

ramukima commented Aug 16, 2016

Wonderful. Many thanks for getting it in so quickly. Few differences though -

  1. It should be fairly easy to do a diff() between two JSON data structures. I see an event raised on fake updates as well. i.e. Say I have a model like below -
list foo {
    key "id";
    leaf id {
        type string;
    }
    leaf name {
        type string;
    }
}

I do a POST to create an item to this list by using the following -

curl-X POST localhost:5050/foo -H 'content-type: application/json' -d '{ "id": 1, "name": "foo1" }'

This generates a 'create' event for the new list entry added as well as an 'update' event on the original foo list, perfect so far 👍

I then attempt to update the "name" attribute of the record added above to "foo2" like below -

curl -X PUT localhost:5050/foo/1 -H 'content-type: application/json' -d '{ "name": "foo2" }'

This generates an 'update' event for "name" field change. Perfect 👍 However, in this case the "prop.path" could indicate the absolute path e.g. "/foo/1/name" instead of "foo/name". Alternatively, the event could inject the "key" of the list entry updated in the event data. Otherwise, I have no way to figure out which record got updated in this case. Does that make sense ?

I then attempt to run the same update by running the same curl command again to check what event is generated (I do not expect any event in this case, as there is actually NO update if a diff was calculated). However, I see an 'update' event on "name" field even though its value did not change. I believe such diff calculator could be very well part of the core event propagation.

Hopefully it all makes sense. Let me know if you believe the notification should behave otherwise.

@sekur
Copy link
Collaborator

sekur commented Aug 16, 2016

No, you're right - I was aware of those shortcomings, I just haven't gotten
to them yet. :-)

As to diff() operation, I'm currently sending the new Property and the old
Property as part of the "update" event (prop, prev). You can perform the
diff via prop.content compared with prev.content. But it would be safer to
do a prop.get() and prev.get() since the prop in question may be an
auto-computed property.

I figured it would be better to send new and old and let the callback
handler figure out how they wanted to handle the diff - if they cared to
handle it.

I can suppress the "update" event if the operation did not result in a data
state change - but that will require implicit diff to take place every time
and I wasn't sure whether the fact that someone attempted to perform an
update (regardless of whether the values actually changed or not) would be
of interest or not.

For example, if you consider a time/refresh type of data where
configuration state is timestamped and the "freshness" of the data is
relevant (regardless of the fact that the data state is essentially the
same), triggering the event may be relevant. I'm open to suggestions on
this, we can even consider a different type of event altogether say "touch"
where it was updated without change?
On Tue, Aug 16, 2016 at 7:33 AM ramukima notifications@github.com wrote:

Wonderful. Many thanks for getting it in so quickly. Few differences
though -

  1. It should be fairly easy to do a diff() between two JSON data
    structures. I see an event raised on fake updates as well. i.e. Say I have
    a model like below -

list foo {
key "id";
leaf id {
type string;
}
leaf name {
type string;
}
}

I do a POST to create an item to this list by using the following -

curl-X POST localhost:5050/foo -H 'content-type: application/json' -d '{ "id": 1, "name": "foo1" }'

This generates a 'create' event for the new list entry added as well as an
'update' event on the original foo list, perfect so far 👍

I then attempt to update the "name" attribute of the record added above to
"foo2" like below -

curl -X PUT localhost:5050/foo/1 -H 'content-type: application/json' -d '{ "name": "foo2" }'

This generates an 'update' event for "name" field change. Perfect 👍
However, in this case the "prop.path" should indicate the absolute path
e.g. "/foo/1/name" instead of "foo/name" because I can not figure out which
record got updated in this case. Does that make sense ?

I then attempt to run the same update by running the same curl command
again to check what event is generated (I do not expect any event in this
case, as there is actually NO update if a diff was calculated). However, I
see an 'update' event on "name" field even though its value did not change.

Hopefully it all makes sense. Let me know if you believe the notification
should behave otherwise.


You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6KbAsWaqvF-5zATsdmiHEChTEPf_zqks5qgcpLgaJpZM4Jj0PH
.

Peter Lee
Corenova Technologies
+1 310 400 6450
peter@corenova.com

@ramukima
Copy link
Contributor Author

It is reasonable to expect an application to perform a diff of changes before taking further actions. I did not notice the (prev, prop) thing earlier, hence my confusion.

Regarding the timestamp on data, I guess that can be integral part of the model even though it may be auto-computed in the backend. Hence, in such situations, an event is expected even when no state in actual data was changed, other than the automatically computed timestamp.

I like the idea of touch events. Also, may be aligning it with NETCONF subscription specification is beneficial as well.

@sekur
Copy link
Collaborator

sekur commented Aug 16, 2016

Sure perfectly reasonable since none of this has been documented. :-)

Need to add Events API section in the README. Probably need to transition
to the wiki module since the README is starting to get rather busy.
On Tue, Aug 16, 2016 at 12:53 PM ramukima notifications@github.com wrote:

It is reasonable to expect an application to perform a diff of changes
before taking further actions. I did not notice the (prev, prop) thing
earlier, hence my confusion.

Regarding the timestamp on data, I guess that can be integral part of the
model even though it may be auto-computed in the backend. Hence, in such
situations, an event is expected even when no state in actual data was
changed, other than the automatically computed timestamp.

I like the idea of touch events. Also, may be aligning it with NETCONF
subscription specification is beneficial as well.


You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6KbCbwD2jbsrQbmgVD7i4QZqEMQC29ks5qghUbgaJpZM4Jj0PH
.

Peter Lee
Corenova Technologies
+1 310 400 6450
peter@corenova.com

sekur added a commit that referenced this issue Aug 17, 2016
…elated to for a list item returning its key
sekur added a commit that referenced this issue Aug 17, 2016
…ed inside the Model (see #25), also added some documentation on 'Model Events'
@sekur
Copy link
Collaborator

sekur commented Aug 17, 2016

I think I've addressed most, if not all of the events API as discussed. Please give the latest version a try and we can close this issue and open more specific ones later as needed. Take a look at the latest documentation - there's more work at hand but I think it's a significant improvement.

@sekur
Copy link
Collaborator

sekur commented Aug 17, 2016

BTW - I've decided against using the wiki since it would be better to have documentation version tagged with the given snapshot being used.

@ramukima
Copy link
Contributor Author

I am somehow still not able to get the filter based event subscription working.

I have two 'on' subscriptions on my model. One without a filter path and other with a filter path. I never see the one with filter specified being called -

pet.on 'update', '/pet/childpet', (prop, item) ->
    console.log "[#{prop.path}] got updated"

pet.on 'update', (prop, prev) ->
    console.log "[#{prop.path}] got updated"

Also, I am not sure if I can specify a filter for 'create' events as well. For example, I want to know when a 'childpet' is created. So not sure if something like this will work -

pet.on 'create', '/pet/childpet', (prop, item) ->
    console.log "childpet created"

I need few examples here.

@sekur
Copy link
Collaborator

sekur commented Aug 19, 2016

I'll take a look - the recent change to internal XPATH processing, etc. may
have broken the granular event subscription.

The unit test suite needs some major update to also include method
validations beyond current set of parse/bind/eval validations.
On Fri, Aug 19, 2016 at 10:41 AM ramukima notifications@github.com wrote:

I am somehow still not able to get the filter based event subscription
working.

I have two 'on' subscriptions on my model. One without a filter path and
other with a filter path. I never see the one with filter specified being
called -

pet.on 'update', '/pet/childpet', (prop, item) ->
console.log "[#{prop.path}] got updated"

pet.on 'update', (prop, prev) ->
console.log "[#{prop.path}] got updated"

Also, I am not sure if I can specify a filter for 'create' events as well.
For example, I want to know when a 'childpet' is created. So not sure if
something like this will work -

pet.on 'create', '/pet/childpet', (prop, item) ->
console.log "childpet created"

I need few examples here.


You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6KbPnlC98L4kubV4VWol3EwvNfaikWks5qherRgaJpZM4Jj0PH
.

Peter Lee
Corenova Technologies
+1 310 400 6450
peter@corenova.com

@sekur
Copy link
Collaborator

sekur commented Aug 19, 2016

@ramukima - sorry I was not able to replicate your issue. It all seems to be working fine...

This is what I tried in the coffee REPL:

~/hack/yang-js 
[master|✔]$ coffee
coffee> schema = 'list pet { key id; leaf id; list childpet { key id; leaf id; } }'
'list pet { key id; leaf id; list childpet { key id; leaf id; } }'
coffee> model = require('./')(schema)()
{}
coffee> model.on 'update', '/pet/childpet', (prop) -> console.log "[#{prop.path}] got updated"
{}
coffee> model.on 'create', '/pet/childpet', (prop) -> console.log "[#{prop.path}] got created"
{}
coffee> model.pet = [ { id: 'one' } ]
[ { id: [Getter/Setter] } ]
coffee> model.pet.one.childpet = [ { id: 'child-one' } ]
[/pet[key() = one]/childpet] got updated
[ { id: [Getter/Setter] } ]
coffee> model.pet.one.childpet.__.merge id: 'child-two'
[/pet[key() = one]/childpet[key() = child-two]] got created
[/pet[key() = one]/childpet[key() = child-two]] got updated
{ name: 'childpet',
  configurable: true,
  enumerable: true,
  set: [Function],
  get: [Function] }
coffee> 

The last action with merge might require a bit of explanation. I'm treating list properties as both an array and a hashmap so that you can do property named based traversal: model.pet.one.childpet

It is able to map the pet.one to be the same as the list-item with pet: { id: 'one' }.

Now model.pet.one.childpet will get you the array of the childpet collection, which is the actual data for that node. In order to access the Property instance for that node, you use __ property to get to the actual Property that is serving that node. So childpet.__ is pointing at the Property instance for the childpet array. When you call childpet.__.merge you are merging a new item into the childpet array collection.

Right now, there is an issue where making direct manipulations to the array instance itself, via .push/.pop, etc. is NOT schema protected. I'm planning on making the array be a copy instead of the real thing when accessed via the Getter so that folks can't accidentally break the schema.

In any case, the above set of REPL commands should reproduce what you'd expect during series of RESTJSON transactions. Please let me know if you run into any other issues.

@sekur
Copy link
Collaborator

sekur commented Aug 19, 2016

One thing to note is that setting the model.pet.one.childpet list with the new array does NOT fire the create event. It only fires the update event for that list collection as a whole - since it is a singular event on the entire collection. Only time create event fires is if that collection is updated with additional items being added into it (such as via merge).

As you've probably noted in the yang-express/restjson module, the @merge operation is used during PATCH/POST. If you perform a PUT on the collection, the create event will NOT fire.

Hope this all makes sense.

@sekur
Copy link
Collaborator

sekur commented Aug 19, 2016

BTW, once you have the initial data set loaded and only interested in listening to a particular element, you can also try this:

model.in('/pet/one/childpet/child-one/id').on 'update', (x) -> console.log "id changed"

This style of event listener will require some prior knowledge on the exact path to traverse, but the benefit is that it will persist across any changes to the id of the childpet or the pet that is holding it. For example, if someone PATCH /pet/one to have a new { id: 'two' }, since the childpet for that record is unaffected, the event listener will still stay relevant.

However, if you used this:

model.on 'update', '/pet/one/childpet/child-one/id`, (x) -> console.log "dynamic path: id changed"

Then it will only apply to any particular instance that happens to be that XPATH expression at any time. So the event listener is actually NOT bound to that matching node, but to the Model itself, such that it will match anytime something has an event that happens to be a match to that XPATH.

@ramukima
Copy link
Contributor Author

Ok, thanks for documenting it nicely. I will try it the way you recommend.

@ramukima
Copy link
Contributor Author

I noticed the paths here you mentioned for the filters is "absolute" XPATH pointing to instances. I was thinking I could do that using YPATH (Yang Path) e.g. on update of /pet/childpet, I could receive events whenever any instances are created/updated/deleted/set/unset under that model.

@sekur
Copy link
Collaborator

sekur commented Aug 21, 2016

Yes, the event listener filter paths apply to events from that "matching" element (and sub-elements).

@ramukima
Copy link
Contributor Author

By the way, I want to mention that "state synschronization" through eventing is a means for external system integration. However, I am skeptical whether thats the right approach for persisting model state changes for the core application.

Imagine I have a event subscription to propagate a list into a data store when new entries in the list get added. When I restart my application, I must fetch that data from the datastore before my application loads completely. Loading such bulk data at once from the persistent datastore in-memory is not advised due to performance and scale reasons.

The best thing would be to introduce "datastore adaptors" my means of "data provider apis". A reference implementation (in-memory, the way it is currently) is provided by the core and applications can hook in other providers when they have the "data provider" APIs implemented.

@sekur
Copy link
Collaborator

sekur commented Aug 22, 2016

Sure, this is how I was planning on addressing this scenario. We can introduce support for Promise wrapped data content. Basically, instead of loading the entire data tree into the in-memory Model instance, we can allow loading of promises for subtree elements such that it is resolved only when it gets accessed (via Getter).

This way, it can still perform necessary schema validations on elements needed as it is being used instead of requiring the entire state to be loaded into memory every time during start.

I think it will help optimize in scenarios where there may be nested lists but if there is a large flat list at the top of the model, it'd still likely require loading most of that data in order to properly transact XPATH predicate operations on it.

@sekur
Copy link
Collaborator

sekur commented Sep 8, 2016

Closing #25, will follow up on improving external synchronization scenario with #38.

@sekur sekur closed this as completed Sep 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants