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

needsActualChange subscription field to implement "always-notify" at subscription side #3439

Closed
fgalan opened this issue Feb 15, 2019 · 11 comments
Assignees
Labels
Milestone

Comments

@fgalan
Copy link
Member

fgalan commented Feb 15, 2019

Issue #3389 covers "always-notify" from the point of view of the update request. However, it is also make sense to set it at subscription level, i.e. a subscription configured with "always-notify" will consider every update operation as an actual update, no matter if the attribute were actually changed or not.

It could be done using a new boolean field needsActualChange within condition. By default, this field is set to true (for backward compatibility reasons). If set to false then all updates associated to that subscription are interpreted always as actual updates, no matter if the attribute were actually changed or not.

Eg:

...
      "condition": {
        "attrs": [
          "temperature "
        ],
        "needsActualChange": false
      }
...
@fgalan fgalan added the backlog label Feb 15, 2019
@alvarosainzpardo
Copy link

This functionality would be very useful for the projects I work on: theese projects are business intelligence dashboards that receive the data using notifications. The source of the data are third-party projects.

@fgalan fgalan pinned this issue Feb 22, 2019
@ghost
Copy link

ghost commented Mar 25, 2019

very good and thanks for this post

@rajeswar-NEC
Copy link
Contributor

Hi @fgalan
I am interested to work on this issue.

@fgalan
Copy link
Member Author

fgalan commented Apr 9, 2021

@rajeswar-NEC thank you for your willingness to work on this! You have been assigned.

@rajeswar-NEC
Copy link
Contributor

rajeswar-NEC commented May 3, 2021

Hi @fgalan

As per #3439 issue, we have to change the notify configurations for below mentioned API’s,
In subscription payload if “needsActualChange” field is set with “true” then it will notify only when “actual update” happens,
If “needsActualChange” field is set with “false” then it will always notify independent of “actual update” happens or not.

Mentioning the some of the requests effect with the above functionality and will notify with respect to "needsActualChange" field.

POST /v2/op/update?options=forcedUpdate
POST /v2/entities/E?options=forcedUpdate
POST /v2/entities/E?options=append,forcedUpdate
PUT /v2/entities/E/attrs/A?options=forcedUpdate
PUT /v2/entities/E/attrs/A/value?options=forcedUpdate
PUT /v2/entities/E?options=forcedUpdate
PATCH /v2/entities/E?options=forcedUpdate

First we have to correctly parse the new field in JSON request/response for subscription management operations of the REST API. For this a new field should be added to the subject class.
The new parameter at JSON API level should be parsed correctly and stored correctly in the DB. A new field name should be defined for that in dbConstants in subscription creation and update operations.

@fgalan
Copy link
Member Author

fgalan commented May 4, 2021

@rajeswar-NEC I think your description above is mostly correct. However, I have one doubt. What do you mean by "Below requests need to be updated" and the list of requests (all them with forcedUpdate)?

@ArqamFarooqui110719
Copy link
Contributor

Hi @fgalan sir,
Due to medical reason @rajeswar-NEC is not able to work on this issue, so I am working on this issue.
As per my investigation, In this issue I have to add the functionality of a new boolean field needsActualChange in subscription payload within subject.condition.
As per this issue, If needsActualChange field will be set to “true” then subscription will notify when only “actual update” happens, and if needsActualChange field will be set to “false” then all updates associated to that subscription are interpreted always as actual updates, no matter if the attribute were actually changed or not. By default, this field will be set to true.

...
      "condition": {
        "attrs": [
          "temperature "
        ],
        "needsActualChange": true/false
      }
...

First this new field needsActualChange will be parsed in parseSubscription() function, and for this a new field should be added in Condition class in Subscription.h.

The new parameter at JSON API level should be parsed correctly and stored correctly in the DB. A new field name should be defined for that in dbConstants in subscription creation and update operations.

After above parts get completed, by using this new field I have to implement it's functionality so that notifications associated to the subscription will be as per this issue.

@fgalan
Copy link
Member Author

fgalan commented Jul 29, 2021

@ArqumFarooqui-NEC thank for your willingness to work on this!

Your analysis is mostly correct. My only comment is that the new field will be stored not only in the DB but also in the csubs cache.

@ArqamFarooqui110719
Copy link
Contributor

Hi @fgalan sir, I have started the fixing this issue and about ~30-40% code had been fixed. But currently I am busy in some other priority tasks since last 1 month approx., Due to which I have put this issue on-hold for sometime and I will resume work on this issue as soon as another task get completed.

@kzangeli
Copy link
Member

FYI, NGSI-LD has always-notify as default (and only) behavior.
So, I had to modify the behavior of not notifying when no real change.

Then an issue arrived:
A client that has two brokers, both subscribing to all of the other broker, needed an option to turn off this behaviour.
If always notifying, any change will provoke an infinite loop of notifications between the two brokers.

So, I implemented "no-notify-on-no-real-change", using a global switch (CLI).
Definitely better to do what you are proposing, let the subscription decide instead of a CLI.
It's on the backlog for NGSI-LD.

Now, to the point:
That same client came back with another issue after implementing what he needed and that new issue was about compound values.
Currently, I imagine Orion doesn't implement the "don't notify if the value hasn't really changed", if the value is a compound.
At least, that's how Orion-LD worked (inherited from Orion) before I fixed it.

The fix isn't difficult, I just rendered the old compound value and the new compound value to two char buffers and then I did a strcmp on them.

@fgalan fgalan unpinned this issue Oct 22, 2021
@fgalan fgalan added this to the 3.8.0 milestone Sep 9, 2022
@fgalan
Copy link
Member Author

fgalan commented Sep 9, 2022

This is covered by the entityUpdate subscription alteration type in issue #1494 in release Orion 3.7.0 (see associated PRs in that issue).

Thus, the issue is going to be closed as solved.

@fgalan fgalan closed this as completed Sep 9, 2022
@fgalan fgalan modified the milestones: 3.8.0, 3.7.0 Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants