Resolve TODO

Resolve Todo:

ngsi-timeseries-api/src/reporter/reporter.py

Line 166 in 6ecedfe

# TODO in this way we return error for even if only one entity

Hi @c0c0n3
I am looking into this issue and have couple of question in my mind,

what is the meaning of wrong entity, as per my understanding if there is missing entity_id or entity_type then this is wrong entity. but error thrown by connection framework.
then how can we find the error, because before coming into notify() it will through error.
can you Please suggest me a way to do this?

I am saying that it is thrown by connection framework because of this line.
https://github.com/orchestracities/ngsi-timeseries-api/blob/master/src/reporter/reporter.py#L87

Gentle reminder!

Hi @NEC-Vishal, sorry for the delay, I'm slowly working through the list of GH notifications :-)

what is the meaning of wrong entity

I think whoever wrote that TODO mean "invalid", i.e. the entity didn;t pass all the validation checks in _validate_payload

as per my understanding if there is missing entity_id or entity_type then this is wrong entity. but error thrown by connection framework.

that's correct, but _validate_payload also checks that each entity in the payload also has at least one attribute other than id and type and that attributes have a value. These Connexion framework doesn't check that---well, more accurately our Swagger spec doesn't specify those constraints, Connexion just validates the input against the spec.

Now onto the TODO. I think the intention of that TODO was to remind us that at the moment if even just a single entity in the payload doesn't pass validation, we throw away the whole payload. A better option would be to split the payload into two lots, valid and invalid entities, insert the valid entities and return a partial failure error with the IDs of the entities that couldn't be inserted.

@c0c0n3 yes we can split the payload into 2 parts, but main problem is how can we find the error, because before coming into notify() it will through error.
if notify() is not call then _validate_payload also not works.
Please guide me.

@NEC-Vishal I think this TODO is not relevant anymore. I'm just looking at the code as it stands now

def notify():
    # ...
    for entity in payload:
        # Validate entity update
        error = _validate_payload(entity)
        if error:
            # TODO in this way we return error for even if only one entity
            #  is wrong
            return error, 400
    # ...

and I think that if error is a dead branch, it'll never happen. In fact, _validate_payload returns an error only if the entity doesn't have id and type attributes, but this check is already done by the connexion framework before it calls notify. This is so because connexion validates the payload against the spec and the spec says the payload should be an array of entities where each entity has both id and type attributes:

Surely, I could've missed something. But maybe you could write a test to check whether I'm right or wrong, then we'll take it from there?