mayankshriv commented on a change in pull request #7071:
URL: https://github.com/apache/incubator-pinot/pull/7071#discussion_r654105233
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/config/TableConfigUtils.java
##########
@@ -235,34 +233,56 @@ public static void
convertFromLegacyTableConfig(TableConfig tableConfig) {
(ingestionConfig != null) ? ingestionConfig.getBatchIngestionConfig()
: null;
SegmentsValidationAndRetentionConfig validationConfig =
tableConfig.getValidationConfig();
+ String segmentPushType = validationConfig.getSegmentPushType();
+ String segmentPushFrequency = validationConfig.getSegmentPushFrequency();
+
if (batchIngestionConfig == null) {
- batchIngestionConfig = new BatchIngestionConfig(null,
validationConfig.getSegmentPushType(),
- validationConfig.getSegmentPushFrequency());
+ // Only create the config if any of the deprecated config is not null.
+ if (segmentPushType != null || segmentPushFrequency != null) {
+ batchIngestionConfig = new BatchIngestionConfig(null, segmentPushType,
segmentPushFrequency);
+ }
} else {
// This should not happen typically, but since we are in repair mode,
might as well cover this corner case.
if (batchIngestionConfig.getSegmentIngestionType() == null) {
-
batchIngestionConfig.setSegmentIngestionType(validationConfig.getSegmentPushType());
+ batchIngestionConfig.setSegmentIngestionType(segmentPushType);
}
if (batchIngestionConfig.getSegmentIngestionFrequency() == null) {
-
batchIngestionConfig.setSegmentIngestionFrequency(validationConfig.getSegmentPushFrequency());
+
batchIngestionConfig.setSegmentIngestionFrequency(segmentPushFrequency);
}
}
StreamIngestionConfig streamIngestionConfig =
(ingestionConfig != null) ? ingestionConfig.getStreamIngestionConfig()
: null;
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+
if (streamIngestionConfig == null) {
- streamIngestionConfig =
- new
StreamIngestionConfig(Collections.singletonList(tableConfig.getIndexingConfig().getStreamConfigs()));
+ Map<String, String> streamConfigs = indexingConfig.getStreamConfigs();
+
+ // Only set the new config if the deprecated one is set.
+ if (streamConfigs != null && !streamConfigs.isEmpty()) {
+ streamIngestionConfig = new
StreamIngestionConfig(Collections.singletonList(streamConfigs));
+ }
}
if (ingestionConfig == null) {
ingestionConfig = new IngestionConfig(batchIngestionConfig,
streamIngestionConfig, null, null, null);
} else {
- ingestionConfig.setBatchIngestionConfig(batchIngestionConfig);
- ingestionConfig.setStreamIngestionConfig(streamIngestionConfig);
+ if (batchIngestionConfig != null) {
+ ingestionConfig.setBatchIngestionConfig(batchIngestionConfig);
+ }
+
+ if (streamIngestionConfig != null) {
+ ingestionConfig.setStreamIngestionConfig(streamIngestionConfig);
+ }
Review comment:
I mean we added an `if` for each line of the code after review, one more
doesn't hurt ;-).
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/config/TableConfigUtils.java
##########
@@ -235,34 +233,56 @@ public static void
convertFromLegacyTableConfig(TableConfig tableConfig) {
(ingestionConfig != null) ? ingestionConfig.getBatchIngestionConfig()
: null;
SegmentsValidationAndRetentionConfig validationConfig =
tableConfig.getValidationConfig();
+ String segmentPushType = validationConfig.getSegmentPushType();
+ String segmentPushFrequency = validationConfig.getSegmentPushFrequency();
+
if (batchIngestionConfig == null) {
- batchIngestionConfig = new BatchIngestionConfig(null,
validationConfig.getSegmentPushType(),
- validationConfig.getSegmentPushFrequency());
+ // Only create the config if any of the deprecated config is not null.
+ if (segmentPushType != null || segmentPushFrequency != null) {
+ batchIngestionConfig = new BatchIngestionConfig(null, segmentPushType,
segmentPushFrequency);
+ }
} else {
// This should not happen typically, but since we are in repair mode,
might as well cover this corner case.
if (batchIngestionConfig.getSegmentIngestionType() == null) {
-
batchIngestionConfig.setSegmentIngestionType(validationConfig.getSegmentPushType());
+ batchIngestionConfig.setSegmentIngestionType(segmentPushType);
}
if (batchIngestionConfig.getSegmentIngestionFrequency() == null) {
-
batchIngestionConfig.setSegmentIngestionFrequency(validationConfig.getSegmentPushFrequency());
+
batchIngestionConfig.setSegmentIngestionFrequency(segmentPushFrequency);
}
}
StreamIngestionConfig streamIngestionConfig =
(ingestionConfig != null) ? ingestionConfig.getStreamIngestionConfig()
: null;
+ IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+
if (streamIngestionConfig == null) {
- streamIngestionConfig =
- new
StreamIngestionConfig(Collections.singletonList(tableConfig.getIndexingConfig().getStreamConfigs()));
+ Map<String, String> streamConfigs = indexingConfig.getStreamConfigs();
+
+ // Only set the new config if the deprecated one is set.
+ if (streamConfigs != null && !streamConfigs.isEmpty()) {
Review comment:
Apparently, the suggested api does not take a Map.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
