walterddr commented on a change in pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#discussion_r723808494
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -303,17 +306,51 @@ public static void validateIngestionConfig(TableConfig
tableConfig, @Nullable Sc
}
}
- private static void validateTaskConfigs(TableConfig tableConfig) {
+ static void validateTaskConfigs(TableConfig tableConfig, Schema schema) {
TableTaskConfig taskConfig = tableConfig.getTaskConfig();
- if (taskConfig != null &&
taskConfig.isTaskTypeEnabled(SEGMENT_GENERATION_AND_PUSH_TASK_TYPE)) {
- Map<String, String> taskTypeConfig =
taskConfig.getConfigsForTaskType(SEGMENT_GENERATION_AND_PUSH_TASK_TYPE);
- if (taskTypeConfig != null && taskTypeConfig.containsKey(SCHEDULE_KEY)) {
- String cronExprStr = taskTypeConfig.get(SCHEDULE_KEY);
- try {
- CronScheduleBuilder.cronSchedule(cronExprStr);
- } catch (Exception e) {
- throw new IllegalStateException(
- String.format("SegmentGenerationAndPushTask contains an invalid
cron schedule: %s", cronExprStr), e);
+ if (taskConfig != null) {
+ for (Map.Entry<String, Map<String, String>> taskConfigEntry :
taskConfig.getTaskTypeConfigsMap().entrySet()) {
+ Map<String, String> taskTypeConfig = taskConfigEntry.getValue();
+ if (taskTypeConfig != null &&
taskTypeConfig.containsKey(SCHEDULE_KEY)) {
+ String cronExprStr = taskTypeConfig.get(SCHEDULE_KEY);
+ try {
+ CronScheduleBuilder.cronSchedule(cronExprStr);
+ } catch (Exception e) {
+ throw new IllegalStateException(String.format(
+ "Task %s contains an invalid cron schedule: %s",
taskConfigEntry.getKey(), cronExprStr), e);
+ }
+ }
+ // Task Specific validation for REALTIME_TO_OFFLINE_TASK_TYPE
+ // TODO task specific validate logic should directly call to
PinotTaskGenerator API
+ if (taskConfigEntry.getKey().equals(REALTIME_TO_OFFLINE_TASK_TYPE)) {
+ if (taskTypeConfig != null) {
+ // check table is not upsert
+ Preconditions.checkState(tableConfig.getUpsertConfig() == null
+ ||
tableConfig.getUpsertConfig().getMode().equals(UpsertConfig.Mode.NONE),
+ "TableConfig cannot have upsert config when using
RealtimeToOfflineTask!");
+ // check no malformed period
+
TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("bufferTimePeriod",
"2d"));
Review comment:
added the default value because it is expected behavior according to the
doc
https://docs.pinot.apache.org/operators/operating-pinot/pinot-managed-offline-flows#config.
but yeah agree. i think we should address this along with the TODO above to
standardized these per-task specific config checkers by adding an interface in
PinotTaskGenerator so that we dont have to duplicate the code
--
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
