kbendick commented on a change in pull request #3561: URL: https://github.com/apache/iceberg/pull/3561#discussion_r766206721
########## File path: core/src/main/java/org/apache/iceberg/rest/http/IcebergHttpResponse.java ########## @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.rest.http; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * All responses for the first version of the Iceberg REST API will be JSON objects, a top-level envelope object with two fields, + * `error` and `data`, which are themselves JSON objects. + * + * data: represents the JSON encoded response for a successful path or for a valid / expected failure. JSON representation of various response types + * error: a standardized obect on error, containing the error code, message, type, and additional metadata (an optional JSON object of metadata) as defined below. + * + * All responses for the REST catalog should be wrapped this way, vs using primitives. For example, for a ListTableResponse, listing tables under a namespace "accounting", + * we'd get a JSON object back like the following: + * + * { "data": { "identifiers": [ "accounting.tax", "accounting.currency_conversions"] }, "error", {} } + * + * If the namesapce `accounting` didn't request, the response from that call would have a body like the following, + * where the `code` 40401 is a two-part identifier: + * - HTTP response code: 404 + * - Two digit internal application defined error code for further detail: 01 - Namespace not found. + * + * { "data": {}, "error": { "message": "Failed to list tables. The Namespace 'accounting' does not exist", "type": "NamespaceNotFoundException", "code": 40401 } + * + * We could also embed the HTTP response code plainly by itself, without internally documented codes, as a separate field. I have found having a documented list of internal + * error codes to be very helpful previously, but am open to discussion on this. Review comment: Ok. Then we'll stick with `type` for now and use the http response codes for `code`. For the moment, there's no hard requirement that they map to exception names. And this class isn't actually part of the spec, it was more to show an idea to somebody. We don't have to use class names / exception names. For now, let's just say that they are a set of well-defined names (at least for any values that we might want the client to react to). Though implementations could in theory use a different set of names and they might have benefit for logging purposes / monitoring purposes, but the client wouldn't be able react to any custom names (because it wouldn't know about them and how to react to them). We don't ideally want to restrict the spec so much that people can't make it suit their needs (for example, if having a richer set of `type` would help them with observability in the future). We can cover what those values are called in a separate discussion later on (this is far from the only discussion / reviewable that will be had about this so I think it's safe to punt it down the road, with that knowledge in mind that that's the general goal for now). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
