-
Notifications
You must be signed in to change notification settings - Fork 14
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
Backend/feature/measurement without event #321
Backend/feature/measurement without event #321
Conversation
…pring2020-cs-gy-9223-class into backend/fetaure/measuerment_without_event
mercury/views/measurement.py
Outdated
} | ||
|
||
for d in dic: | ||
if json_data.get(dic[d]) is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: name suggestion for readability
key_map = {
"timestamp": "date",
"sensor_id": "sensor_id",
"value": "values",
}
for key, json_key in key_map.items():
if json_key not in json_data:
return Response(...)
res[key] = json_data[json_key]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice suggestion! I have updated the code.
@@ -12,12 +12,42 @@ def build_error(str): | |||
return json.dumps({"error": str}) | |||
|
|||
|
|||
def add_measurement(request, event): | |||
json_data = request.data | |||
if isinstance(json_data, str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if json_data is not str?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be a JSON dict. The only case that the json_data is a string is that it is transferred through radio script since the native request module can only accept the string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Later it would be great to add assert for the input type (i.e either a JSON dict or a stringified json, otherwise fail loudly). But looks good to me at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Take the first event as the active event for end to end demo