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

Possible fix for multiple data elements in notifications not supported yet #185

Closed
ohylli opened this issue May 6, 2019 · 2 comments
Closed
Assignees

Comments

@ohylli
Copy link
Contributor

ohylli commented May 6, 2019

I was attempting to find the most efficient way to send a lot of updates to QuantumLeap and so decided to use the notify api end point directly. Then I noticed that it only supports notifications containing only one element in the data array. If more than one element is given a response with status 400 and content "Multiple data elements in notifications not supported yet" is returned. I started to explore the code and found out that adding support for multiple elements was quite simple. I wonder if there is something I am missing here and thus decided to first create an issue about this before making a full pull request with tests and everrything.

My modifications were based on the latest commit
in the v0.6.1rc branch. I modified the notify function in reporter.py
to loop through every element in the data array to perform all operations done to the single payload before (validation, adding the time index etc.). This list of entity updates is then given to the insert method of the CrateTranslatorInstance. In crate.py
I modified the _preprocess_values method to add None to the values list if the entity update does not have a value for an attribute for which some other entity update of the same type in the notification had a value. This then seems to cause null to be inserted to the database as the value for the attribute. The current implementation throws an NotImplementedError with the message "Seems like not all entities of same type came with the same set of attributes".

With these modifications notifications with multiple updates seem to work all right: the data is saved correctly to crate and returned correctly when queried through the QuantumLeap API. All tests also pass except the one that specifically tests that multiple elements in the notification data gives a response with status 400. Since this was so easy because the implementation almost is there already I started to wonder why it was not yet implemented and if there actually is something wrong with my solution and the correct way to fix this is more complicated.

@chicco785
Copy link
Contributor

dear @ohylli
thanks for looking into quantum leap :)
most probably it was not implemented since at afaik, orion return 1 data element per notification, but i may be wrong :) @taliaga ?

cheers,
federico

@chicco785
Copy link
Contributor

@ohylli we discussed in our weekly sprint. we assumed multiple data notification where never issued by orion. which is not the case in a special case, the initial notification:
https://fiware-orion.readthedocs.io/en/master/user/initial_notification/index.html

We welcome a pull request on the matter!

Thanks!

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

3 participants