Skip to content
This repository has been archived by the owner on Oct 18, 2023. It is now read-only.

Add HTTP API v1 ("Hrana over HTTP") #305

Merged
merged 3 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Implement batches in Hrana over HTTP
  • Loading branch information
honzasp committed Mar 25, 2023
commit e2bd4d2d8418c6cbda4e311cfea69834de68d1c4
44 changes: 43 additions & 1 deletion sqld/src/http/hrana_over_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ enum ResponseError {

#[error(transparent)]
Stmt(batch::StmtError),
#[error(transparent)]
Batch(batch::BatchError),
}

pub async fn handle_index(
Expand Down Expand Up @@ -62,6 +64,43 @@ pub async fn handle_execute(
})
}

pub async fn handle_batch(
db_factory: Arc<dyn DbFactory>,
req: hyper::Request<hyper::Body>,
) -> Result<hyper::Response<hyper::Body>> {
#[derive(Debug, Deserialize)]
struct ReqBody {
batch: batch::proto::Batch,
}

#[derive(Debug, Serialize)]
struct RespBody {
result: batch::proto::BatchResult,
}

let res: Result<_> = async move {
let req_body = json_request_body::<ReqBody>(req.into_body()).await?;
let db = db_factory
.create()
.await
.context("Could not create a database connection")?;
let result = batch::execute_batch(&*db, &req_body.batch)
.await
.map_err(|err| match err.downcast::<batch::BatchError>() {
Ok(batch_err) => anyhow!(ResponseError::Batch(batch_err)),
Err(err) => err,
})
.context("Could not execute batch")?;
Ok(json_response(hyper::StatusCode::OK, &RespBody { result }))
}
.await;

Ok(match res {
Ok(resp) => resp,
Err(err) => error_response(err.downcast::<ResponseError>()?),
})
}

async fn json_request_body<T: DeserializeOwned>(body: hyper::Body) -> Result<T> {
let body = hyper::body::to_bytes(body).await?;
let body =
Expand All @@ -70,7 +109,7 @@ async fn json_request_body<T: DeserializeOwned>(body: hyper::Body) -> Result<T>
}

fn error_response(err: ResponseError) -> hyper::Response<hyper::Body> {
use batch::StmtError;
use batch::{BatchError, StmtError};
let status = match &err {
ResponseError::BadRequestBody { .. } => hyper::StatusCode::BAD_REQUEST,
ResponseError::Stmt(err) => match err {
Expand All @@ -85,6 +124,9 @@ fn error_response(err: ResponseError) -> hyper::Response<hyper::Body> {
}
StmtError::SqliteError { .. } => hyper::StatusCode::INTERNAL_SERVER_ERROR,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm regarding my comment before: I think this is the culprit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then perhaps we should map all errors to HTTP status 400?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure
let's start with that and work on the error story later

},
ResponseError::Batch(err) => match err {
BatchError::CondBadStep => hyper::StatusCode::BAD_REQUEST,
},
};

json_response(
Expand Down
1 change: 1 addition & 0 deletions sqld/src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ async fn handle_request(
(&Method::GET, "/health") => Ok(handle_health()),
(&Method::GET, "/v1") => hrana_over_http::handle_index(req).await,
(&Method::POST, "/v1/execute") => hrana_over_http::handle_execute(db_factory, req).await,
(&Method::POST, "/v1/batch") => hrana_over_http::handle_batch(db_factory, req).await,
_ => Ok(Response::builder().status(404).body(Body::empty()).unwrap()),
}
}
Expand Down