Standardisation of rest apis #847
Conversation
|
/assign @axsaucedo |
|
/assign @cliveseldon |
|
/unassign @cliveseldon |
|
/cc @cliveseldon |
|
/uncc @arnaudvl |
|
@axsaucedo should the changes for the feedback API be done in a different PR? |
|
@jklaise my thoughts were to keep the feedback as is, as the only reason we're modifying the rest of the APIs is due to the requirement for proxy servers (TFserving and potentially others). The feedback API currently doesn't seem to require this functionality, and feedback may be different as we implement the stateful metrics piece. I think for now perhaps as you say we can create an issue to address this in a different PR if required. What are your thoughts. |
|
There is another side effect which was solved for the /predict endpoint and this PR solves for others (apart from /send-feedback) - default values of metrics were missing, i.e. if a metric has a value of 0, protobuf conversion interprets it as missing and returns the metric e.g. as {"key":"accuracy", "type": "GAUGE} when it should be {"key":"accuracy", "type": "GAUGE, "value": 0}, this will need to be fixed for the /send-feedback endpoint too for the stateful metrics. But I agree that this could be a separate PR possibly with additional functionality. |
|
I think you will need to rebase for this to be mergable. |
|
/lgtm |
|
New changes are detected. LGTM label has been removed. |
|
Thanks @jklaise |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: axsaucedo, jklaise The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #739
When a REST request gets parsed into a Proto, all the integers get converted to floats, which ends up changing the requests. This was fixed for the /predict endpoint in #707. This has now been done with all the APIs except the feedback API as that uses a different GRPC object.