Jackie-Jiang commented on code in PR #12440:
URL: https://github.com/apache/pinot/pull/12440#discussion_r1554429709
##########
pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java:
##########
@@ -287,4 +337,46 @@ private static PropertiesConfiguration
createPropertiesConfiguration(boolean set
return config;
}
+
+ /**
+ * Creates the IOFactory based on the provided kind.
+ *
+ * @param ioFactoryKind IOFactory kind
+ * @return IOFactory
+ */
+ private static IOFactory createPropertyIOFactory(PropertyIOFactoryKind
ioFactoryKind) {
+ switch (ioFactoryKind) {
+ case ConfigFileIOFactory:
+ return new ConfigFilePropertyIOFactory();
+ case SegmentMetadataIOFactory:
+ return new SegmentMetadataPropertyIOFactory();
+ default:
+ return new PropertiesConfiguration.DefaultIOFactory();
+ }
+ }
+
+ /**
+ * checks whether the segment metadata file first line is version header or
not.
+ * @param segmentMetadataFile segment metadata file
+ * @return boolean
+ * @throws ConfigurationException exception.
+ */
+ private static boolean checkHeaderExistsInSegmentMetadata(File
segmentMetadataFile)
Review Comment:
Can we make this method return the version number of the file? Then we can
have different writer/reader pair for different version number. In the current
PR we treat `DefaultPropertyConfigurationIOFactory` as v1, and
`SegmentMetadataIOFactory` as v2 which is not explicit. Consider grouping them
with version number.
--
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: commits-unsubscr...@pinot.apache.org
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
