amogh-jahagirdar commented on code in PR #13191:
URL: https://github.com/apache/iceberg/pull/13191#discussion_r2121603778


##########
core/src/main/java/org/apache/iceberg/rest/BaseHTTPClient.java:
##########
@@ -77,6 +78,18 @@ public <T extends RESTResponse> T get(
     return execute(request, responseType, errorHandler, h -> {});
   }
 
+  @Override
+  public <T extends RESTResponse> T get(
+      String path,
+      Map<String, String> queryParams,
+      Class<T> responseType,
+      Map<String, String> headers,
+      Consumer<ErrorResponse> errorHandler,
+      InjectableValues injectableValues) {

Review Comment:
   I think it'd be nice if we can hide the implementation detail of Jackson 
`InjectableValues` here.
   
    Is there a way for us to define a `ParserContext` (or maybe there's a 
better name), and one of the methods on that interface is a 
`toInjectableValues()` which gets invoked in `HttpClient`. But the methods 
which changed would change to accept a `ParserContext` instead. 



##########
core/src/main/java/org/apache/iceberg/rest/BaseHTTPClient.java:
##########
@@ -77,6 +78,18 @@ public <T extends RESTResponse> T get(
     return execute(request, responseType, errorHandler, h -> {});
   }
 
+  @Override
+  public <T extends RESTResponse> T get(
+      String path,
+      Map<String, String> queryParams,
+      Class<T> responseType,
+      Map<String, String> headers,
+      Consumer<ErrorResponse> errorHandler,
+      InjectableValues injectableValues) {

Review Comment:
   Or even more generic, I think at the end of the day a `ParserContext` is 
really just a containing a `Map<String, Object>`. The injectable value can 
completely be hidden in the implementation of `RestClient` when setting it on 
the object reader
   
   `ParserContext.builder().add("specsBydId", specsById).add().build()`.



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