apucher commented on a change in pull request #6613:
URL: https://github.com/apache/incubator-pinot/pull/6613#discussion_r593503297
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java
##########
@@ -34,29 +34,40 @@
public class SegmentFetcherFactory {
- private SegmentFetcherFactory() {
- }
+ private final static SegmentFetcherFactory instance = new
SegmentFetcherFactory();
static final String SEGMENT_FETCHER_CLASS_KEY_SUFFIX = ".class";
private static final String PROTOCOLS_KEY = "protocols";
+ private static final String AUTH_TOKEN_KEY = "auth.token";
private static final String ENCODED_SUFFIX = ".enc";
private static final Logger LOGGER =
LoggerFactory.getLogger(SegmentFetcherFactory.class);
- private static final Map<String, SegmentFetcher> SEGMENT_FETCHER_MAP = new
HashMap<>();
- private static final SegmentFetcher DEFAULT_HTTP_SEGMENT_FETCHER = new
HttpSegmentFetcher();
- private static final SegmentFetcher DEFAULT_PINOT_FS_SEGMENT_FETCHER = new
PinotFSSegmentFetcher();
-
- static {
Review comment:
The use of this class is pretty messy. The fetchers here were
instantiated statically without configuration, but we now may need to inject
authTokens. I've therefore refactored the class to maintain its public (static)
interface, while setting properties in the starter before use.
With my manual testing, quickstart, and integration tests things seem to
work, but I bet there;s a corner case lurking somewhere.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
