Open pooja1pathak opened 1 year ago
Hi @c0c0n3 I am looking into this issue and have couple of question in my mind,
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?
Resolve Todo: https://github.com/orchestracities/ngsi-timeseries-api/blob/6ecedfe5e81e346fc02e3cbd0bc86e6c20fab25a/src/reporter/reporter.py#L166