connortsui20 commented on code in PR #965:
URL: https://github.com/apache/iceberg-rust/pull/965#discussion_r1957524251
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -670,21 +719,47 @@ impl Catalog for RestCatalog {
})
.build()?;
- let resp = self
- .context()
- .await?
- .client
- .query::<CommitTableResponse, ErrorResponse>(request)
- .await?;
+ let handler = |response: Response| async move {
+ if response.status() == StatusCode::OK {
+ return
deserialize_catalog_response::<CommitTableResponse>(response).await;
+ }
+
+ let error_message = match response.status() {
+ StatusCode::NOT_FOUND => "Tried to update a table that does
not exist",
+ StatusCode::CONFLICT => {
+ "CommitFailedException, one or more requirements failed.
The client may retry."
+ }
+ StatusCode::INTERNAL_SERVER_ERROR => {
+ "An unknown server-side problem occurred; the commit state
is unknown."
+ }
+ StatusCode::BAD_GATEWAY => {
+ "A gateway or proxy received an invalid response from the
upstream server; the commit state is unknown."
+ }
+ StatusCode::GATEWAY_TIMEOUT => {
+ "A server-side gateway timeout occurred; the commit state
is unknown."
+ }
+ _ => {
+ return Err(deserialize_unexpected_catalog_error(
+ response,
+ )
+ .await);
+ }
+ };
+
+ Err(Error::new(ErrorKind::Unexpected, error_message))
+ };
+
+ let response = context.client.query_catalog(request, handler).await?;
Review Comment:
> Ok, the client will perform query and returns a response or error.
I agree that this is clearer, but the whole point of this PR is that by
having a single `query` function you are unable to handle each API call's
response specifically. There will be cases when only allowing `OK` or
`NO_CONTENT` is not enough: for example in the `loadTable` response it should
be able to handle `StatusCode::NOT_MODIFIED` (304) as a correct case. There
should have to be a new kind of `query` or `execute` function for every case
that needs to be handled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
