Add HTTP API v1 ("Hrana over HTTP") #305
Conversation
There was a problem hiding this comment.
@honzasp
I tried executing: echo '{ "stmt": {"sql": "select * from test", "want_rows": true }}' | http post "127.0.0.1:8080/v1/execute"
The returned error is a 500, and gets logged in the logs:
2023-03-28T11:46:36.206182Z INFO sqld::http: got request: POST /v1/execute
2023-03-28T11:46:36.206974Z ERROR tower_http::trace::on_failure: response failed classification=Status code: 500 Internal Server Error latency=0 ms
I am not sure this is adequate, and further error categorization should probably be performed
| StmtError::TransactionTimeout | StmtError::TransactionBusy => { | ||
| hyper::StatusCode::SERVICE_UNAVAILABLE | ||
| } | ||
| StmtError::SqliteError { .. } => hyper::StatusCode::INTERNAL_SERVER_ERROR, |
There was a problem hiding this comment.
Uhm regarding my comment before: I think this is the culprit
There was a problem hiding this comment.
What HTTP status would be more appropriate? The error response includes a detailed message and will soon include the error code, the HTTP status is not very important.
There was a problem hiding this comment.
I was just surprised because it came up in the logs as an internal error, which leads me to believe that there is something broken, although everything is fine, and this is simply a user error
There was a problem hiding this comment.
Ok, then perhaps we should map all errors to HTTP status 400?
There was a problem hiding this comment.
not sure
let's start with that and work on the error story later
|
bors merge |
|
Build succeeded: |
We need to update the HTTP API to support batches and to fix some issues (such as encoding of types). Instead of coming up with a new API, let's just reuse the Hrana structures and semantics. In effect, the new version of the HTTP API is "Hrana over HTTP".
This is needed for the new generation of the client libraries.