adutra commented on code in PR #18756:
URL: https://github.com/apache/iceberg/pull/18756#discussion_r4471787947


##########
core/src/main/java/org/apache/iceberg/rest/RESTClient.java:
##########
@@ -226,4 +226,14 @@ <T extends RESTResponse> T postForm(
   default RESTClient withAuthSession(AuthSession session) {
     return this;
   }
+
+  /**
+   * Unwraps this client to the given type, if possible. This allows callers 
to access the
+   * underlying HTTP library used by this client.
+   *
+   * @return the unwrapped client, or null if this client cannot be unwrapped 
to the given type
+   */
+  default <T> T unwrap(Class<T> type) {
+    return type.isInstance(this) ? type.cast(this) : null;
+  }

Review Comment:
   There is one alternative to `unwrap()`, but it also has some drawbacks: we 
could introduce a general-purpose `RESTClient.send()` method:
   
   ```java
     default <T> T send(
         HTTPRequest request,
         Class<T> responseType,
         Consumer<ErrorResponse> errorHandler,
         Consumer<Map<String, String>> responseHeaders) {
       throw new UnsupportedOperationException();
     }
   ```
   
   Since that method returns an arbitrary type T (instead of `RESTResponse`), 
it would work with Nimbus response types too.
   
   But the method looks a bit odd compared to the other methods in 
`RESTClient`, because it's a lot more "generic" that the other ones. That's why 
I went with `unwrap()`.
   
   If you prefer the `send()` approach, I'm happy to make the necessary changes.



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

Reply via email to