Conversation
vishakha041
left a comment
There was a problem hiding this comment.
There is a mismatch in whether blob is true by default. Please fix either the wiki or the code. Rest seems fine with some comments.
|
|
||
| for (auto ent : entities) { | ||
| if (ent["_id"].asInt64() == d_id) { | ||
| if (ent[VDMS_DESC_ID_PROP].asInt64() == d_id) { |
There was a problem hiding this comment.
Seems like some cleanup mixed in with the actual changes. Just make sure they aren't numerous enough to warrant a separate commit. Otherwise its ok.
There was a problem hiding this comment.
This is actually needed here in this commit as the "convertproperties" function (that was doing this translation internally) was moved below.
src/DescriptorsCommand.cc
Outdated
| const Json::Value& set_response = json_responses[0]; | ||
| const Json::Value& set = set_response["entities"][0]; | ||
|
|
||
| // This properties should always exist |
|
|
||
| if (findDesc.isMember("entities")) { | ||
|
|
||
| if (get_value<bool>(results, "blob", false)) { |
There was a problem hiding this comment.
Our wiki page says that blob is assumed true by default. Here it seems false is default. Which one needs correction?
There was a problem hiding this comment.
Given that descriptors usually are a proxy for retrieving other objects, we decided some time back that the default behavior would be not to return the blob by default. Need a wiki update.
fix hadolint summary
Fix #43 and #91