danielcweeks commented on code in PR #11992:
URL: https://github.com/apache/iceberg/pull/11992#discussion_r1926121826


##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -214,92 +225,63 @@ private static void throwFailure(
     throw new RESTException("Unhandled error: %s", errorResponse);
   }
 
-  private URI buildUri(String path, Map<String, String> params) {
-    // if full path is provided, use the input path as path
-    if (path.startsWith("/")) {
-      throw new RESTException(
-          "Received a malformed path for a REST request: %s. Paths should not 
start with /", path);
-    }
-    String fullPath =
-        (path.startsWith("https://";) || path.startsWith("http://";))
-            ? path
-            : String.format("%s/%s", uri, path);
-    try {
-      URIBuilder builder = new URIBuilder(fullPath);
-      if (params != null) {
-        params.forEach(builder::addParameter);
-      }
-      return builder.build();
-    } catch (URISyntaxException e) {
-      throw new RESTException(
-          "Failed to create request URI from base %s, params %s", fullPath, 
params);
-    }
-  }
-
-  /**
-   * Method to execute an HTTP request and process the corresponding response.
-   *
-   * @param method - HTTP method, such as GET, POST, HEAD, etc.
-   * @param queryParams - A map of query parameters
-   * @param path - URI to send the request to
-   * @param requestBody - Content to place in the request body
-   * @param responseType - Class of the Response type. Needs to have 
serializer registered with
-   *     ObjectMapper
-   * @param errorHandler - Error handler delegated for HTTP responses which 
handles server error
-   *     responses
-   * @param <T> - Class type of the response for deserialization. Must be 
registered with the
-   *     ObjectMapper.
-   * @return The response entity, parsed and converted to its type T
-   */
-  private <T> T execute(
-      Method method,
+  @Override
+  protected HTTPRequest buildRequest(
+      HTTPMethod method,
       String path,
       Map<String, String> queryParams,
-      Object requestBody,
-      Class<T> responseType,
       Map<String, String> headers,
-      Consumer<ErrorResponse> errorHandler) {
-    return execute(
-        method, path, queryParams, requestBody, responseType, headers, 
errorHandler, h -> {});
+      Object body) {
+
+    ImmutableHTTPRequest.Builder builder =
+        ImmutableHTTPRequest.builder()
+            .baseUri(baseUri)
+            .mapper(mapper)
+            .method(method)
+            .path(path)

Review Comment:
   It appears that we may have dropped some of the path validation (e.g. 
asserting that paths don't start with a slash)



-- 
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

Reply via email to