sunithabeeram commented on a change in pull request #4243: Add support for
passing headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r289029741
##########
File path:
pinot-api/src/main/java/org/apache/pinot/client/PinotClientTransportFactory.java
##########
@@ -18,9 +18,11 @@
*/
package org.apache.pinot.client;
+import java.util.Map;
+
/**
* Factory for client transports.
*/
interface PinotClientTransportFactory {
- PinotClientTransport buildTransport();
+ PinotClientTransport buildTransport(Map<String, String> headers);
Review comment:
The code structure seems a bit weird to me. If ConnectionFactory is the one
that can be extended, then shouldn't that actually be an interface and made
Public.Stable? Its odd that we have a specific implementation there and expect
clean extensibility.
Given the lack of clarity, it would be great if the javadocs for
ConnectionFactory and PinotClientConnectionFactory be updated to expand on how
they should be used - ie, what should change when new transports are
considered/added. @kishoreg, hope you can provide appropriate input here for
Jitendra.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
