orchestracities / ngsi-timeseries-api

QuantumLeap: a FIWARE Generic Enabler to support the usage of NGSIv2 (and NGSI-LD experimentally) data in time-series databases
https://quantumleap.rtfd.io/
MIT License
38 stars 49 forks source link

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

Closed ohylli closed 5 years ago

ohylli commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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