You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/10/05 17:04:04 UTC
[GitHub] [pinot] walterddr opened a new pull request #7523: add validation for realtimeToOffline task
walterddr opened a new pull request #7523:
URL: https://github.com/apache/pinot/pull/7523
Based on
https://docs.pinot.apache.org/operators/operating-pinot/pinot-managed-offline-flows
Currently the task configs are not parsed until task generation, which should be validated sooner. Adding a validation for realtimeToOffline task
However a more general solution should be:
1. add validateConfig method to PinotTaskGenerator
2. make validateTaskConfigs reflectively call validateConfig provided by the concrete impl of PinotTaskGenerator
This way we don't really need to duplicate config checkers between PinotTaskGenerator impl and within TableConfigUtils. (such as the task name copy and all other config parsing logic)
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#discussion_r723759353
##########
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)) {
Review comment:
```suggestion
if (taskTypeConfig.containsKey(SCHEDULE_KEY)) {
```
##########
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) {
Review comment:
Remove
##########
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"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("bucketTimePeriod", "1d"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("roundBucketTimePeriod", "1s"));
+ // check mergeType is correct
+ Preconditions.checkState(ImmutableSet.of("concat", "rollup", "dedup").contains(
+ taskTypeConfig.getOrDefault("mergeType", "concat")),
+ "MergeType must be one of [concat, rollup, dedup]!");
Review comment:
(Major) The check should be case-insensitive. We usually prefer all capital for enum
```suggestion
Preconditions.checkState(ImmutableSet.of("CONCAT", "ROLLUP", "DEDUP").contains(
taskTypeConfig.getOrDefault("mergeType", "CONCAT").toUpperCase()),
"MergeType must be one of [CONCAT, ROLLUP, DEDUP]!");
```
##########
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) {
Review comment:
Annotate with `@VisibleForTesting`
##########
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:
Suggest wrapping this into a helper method, and only check when the config exists. No need to validate the default value
##########
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"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("bucketTimePeriod", "1d"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("roundBucketTimePeriod", "1s"));
+ // check mergeType is correct
+ Preconditions.checkState(ImmutableSet.of("concat", "rollup", "dedup").contains(
+ taskTypeConfig.getOrDefault("mergeType", "concat")),
+ "MergeType must be one of [concat, rollup, dedup]!");
+ // check no mis-configured columns
+ Set<String> columnNames = schema.getColumnNames();
+ for (Map.Entry<String, String> entry : taskTypeConfig.entrySet()) {
+ if (entry.getKey().endsWith(".aggregationType")) {
+ Preconditions.checkState(
+ columnNames.contains(StringUtils.removeEnd(entry.getKey(), ".aggregationType")),
+ String.format("Column \"%s\" not found in schema!", entry.getKey()));
+ Preconditions.checkState(ImmutableSet.of("sum", "max", "min").contains(entry.getValue()),
Review comment:
Same here, case-insensitive check
##########
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
Review comment:
This is equivalent to the check on line 351. We can remove the check here
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a change in pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
walterddr commented on a change in pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#discussion_r722667729
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -303,17 +307,40 @@ public static void validateIngestionConfig(TableConfig tableConfig, @Nullable Sc
}
}
- private static void validateTaskConfigs(TableConfig tableConfig) {
+ private 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);
+ // TODO validate task config directly from PinotTaskGenerator API
+ if (taskConfig != null) {
+ if (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.isTaskTypeEnabled(REALTIME_TO_OFFLINE_TASK_TYPE)) {
+ Map<String, String> taskTypeConfig = taskConfig.getConfigsForTaskType(REALTIME_TO_OFFLINE_TASK_TYPE);
+ if (taskTypeConfig != null) {
Review comment:
no introduced by this PR either but will fix. Also would add some test before merging.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a change in pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75c8e78) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `38.45%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
=============================================
- Coverage 70.75% 32.30% -38.46%
=============================================
Files 1523 1514 -9
Lines 75540 75325 -215
Branches 11007 10999 -8
=============================================
- Hits 53452 24333 -29119
- Misses 18422 48894 +30472
+ Partials 3666 2098 -1568
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.67% <0.00%> (?)` | |
| integration2 | `29.09% <0.00%> (+0.03%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (-66.14%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/QuotaConfig.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1F1b3RhQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1051 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...75c8e78](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ca33f55) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `4.88%`.
> The diff coverage is `86.66%`.
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
============================================
- Coverage 70.75% 65.87% -4.89%
- Complexity 3400 3410 +10
============================================
Files 1523 1477 -46
Lines 75540 73791 -1749
Branches 11007 10834 -173
============================================
- Hits 53452 48609 -4843
- Misses 18422 21723 +3301
+ Partials 3666 3459 -207
```
| Flag | Coverage Δ | |
|---|---|---|
| integration2 | `?` | |
| unittests1 | `69.88% <86.66%> (+0.03%)` | :arrow_up: |
| unittests2 | `15.26% <0.00%> (+0.74%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `69.19% <86.66%> (+3.05%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ot/common/restlet/resources/TableMetadataInfo.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVGFibGVNZXRhZGF0YUluZm8uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [330 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...ca33f55](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a174f2e) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `38.40%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
=============================================
- Coverage 70.75% 32.35% -38.41%
=============================================
Files 1523 1514 -9
Lines 75540 75325 -215
Branches 11007 10999 -8
=============================================
- Hits 53452 24375 -29077
- Misses 18422 48851 +30429
+ Partials 3666 2099 -1567
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.45% <0.00%> (?)` | |
| integration2 | `29.02% <0.00%> (-0.05%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (-66.14%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/QuotaConfig.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1F1b3RhQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1051 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...a174f2e](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75c8e78) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `41.66%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
=============================================
- Coverage 70.75% 29.09% -41.67%
=============================================
Files 1523 1514 -9
Lines 75540 75325 -215
Branches 11007 10999 -8
=============================================
- Hits 53452 21917 -31535
- Misses 18422 51408 +32986
+ Partials 3666 2000 -1666
```
| Flag | Coverage Δ | |
|---|---|---|
| integration2 | `29.09% <0.00%> (+0.03%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (-66.14%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/QuotaConfig.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1F1b3RhQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1018 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...75c8e78](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (196e6c3) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `5.83%`.
> The diff coverage is `57.14%`.
> :exclamation: Current head 196e6c3 differs from pull request most recent head 47f17eb. Consider uploading reports for the commit 47f17eb to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
============================================
- Coverage 70.75% 64.92% -5.84%
+ Complexity 3400 3325 -75
============================================
Files 1523 1514 -9
Lines 75540 75284 -256
Branches 11007 10994 -13
============================================
- Hits 53452 48879 -4573
- Misses 18422 22922 +4500
+ Partials 3666 3483 -183
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.61% <41.26%> (?)` | |
| integration2 | `29.05% <5.55%> (-0.01%)` | :arrow_down: |
| unittests1 | `69.82% <42.00%> (-0.02%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ent/index/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `82.96% <0.00%> (ø)` | |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `63.91% <4.34%> (-2.22%)` | :arrow_down: |
| [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `81.10% <16.66%> (-1.44%)` | :arrow_down: |
| [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | `75.45% <62.85%> (-17.54%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==) | `83.33% <100.00%> (+5.69%)` | :arrow_up: |
| [...troller/helix/core/minion/ClusterInfoAccessor.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9DbHVzdGVySW5mb0FjY2Vzc29yLmphdmE=) | `75.86% <100.00%> (+15.86%)` | :arrow_up: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `28.35% <100.00%> (+5.74%)` | :arrow_up: |
| [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.08% <100.00%> (+7.00%)` | :arrow_up: |
| [...pinot/controller/recommender/io/ConfigManager.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9Db25maWdNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...troller/recommender/io/metadata/FieldMetadata.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9tZXRhZGF0YS9GaWVsZE1ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [271 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...47f17eb](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#discussion_r723759353
##########
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)) {
Review comment:
```suggestion
if (taskTypeConfig.containsKey(SCHEDULE_KEY)) {
```
##########
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) {
Review comment:
Remove
##########
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"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("bucketTimePeriod", "1d"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("roundBucketTimePeriod", "1s"));
+ // check mergeType is correct
+ Preconditions.checkState(ImmutableSet.of("concat", "rollup", "dedup").contains(
+ taskTypeConfig.getOrDefault("mergeType", "concat")),
+ "MergeType must be one of [concat, rollup, dedup]!");
Review comment:
(Major) The check should be case-insensitive. We usually prefer all capital for enum
```suggestion
Preconditions.checkState(ImmutableSet.of("CONCAT", "ROLLUP", "DEDUP").contains(
taskTypeConfig.getOrDefault("mergeType", "CONCAT").toUpperCase()),
"MergeType must be one of [CONCAT, ROLLUP, DEDUP]!");
```
##########
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) {
Review comment:
Annotate with `@VisibleForTesting`
##########
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:
Suggest wrapping this into a helper method, and only check when the config exists. No need to validate the default value
##########
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"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("bucketTimePeriod", "1d"));
+ TimeUtils.convertPeriodToMillis(taskTypeConfig.getOrDefault("roundBucketTimePeriod", "1s"));
+ // check mergeType is correct
+ Preconditions.checkState(ImmutableSet.of("concat", "rollup", "dedup").contains(
+ taskTypeConfig.getOrDefault("mergeType", "concat")),
+ "MergeType must be one of [concat, rollup, dedup]!");
+ // check no mis-configured columns
+ Set<String> columnNames = schema.getColumnNames();
+ for (Map.Entry<String, String> entry : taskTypeConfig.entrySet()) {
+ if (entry.getKey().endsWith(".aggregationType")) {
+ Preconditions.checkState(
+ columnNames.contains(StringUtils.removeEnd(entry.getKey(), ".aggregationType")),
+ String.format("Column \"%s\" not found in schema!", entry.getKey()));
+ Preconditions.checkState(ImmutableSet.of("sum", "max", "min").contains(entry.getValue()),
Review comment:
Same here, case-insensitive check
##########
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
Review comment:
This is equivalent to the check on line 351. We can remove the check here
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fcf19c8) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **increase** coverage by `1.01%`.
> The diff coverage is `76.29%`.
> :exclamation: Current head fcf19c8 differs from pull request most recent head 3411145. Consider uploading reports for the commit 3411145 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
============================================
+ Coverage 70.75% 71.77% +1.01%
- Complexity 3400 3411 +11
============================================
Files 1523 1523
Lines 75540 75642 +102
Branches 11007 11034 +27
============================================
+ Hits 53452 54290 +838
+ Misses 18422 17643 -779
- Partials 3666 3709 +43
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.65% <40.00%> (?)` | |
| integration2 | `?` | |
| unittests1 | `69.87% <80.70%> (+0.03%)` | :arrow_up: |
| unittests2 | `15.26% <19.25%> (+0.74%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ent/index/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `82.96% <0.00%> (ø)` | |
| [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `74.80% <16.66%> (-7.74%)` | :arrow_down: |
| [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | `87.72% <68.57%> (-5.27%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `69.19% <86.66%> (+3.05%)` | :arrow_up: |
| [...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==) | `80.86% <100.00%> (+3.22%)` | :arrow_up: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `92.45% <100.00%> (+0.29%)` | :arrow_up: |
| [...troller/helix/core/minion/ClusterInfoAccessor.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9DbHVzdGVySW5mb0FjY2Vzc29yLmphdmE=) | `89.65% <100.00%> (+29.65%)` | :arrow_up: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `65.51% <100.00%> (+42.91%)` | :arrow_up: |
| [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.08% <100.00%> (+7.00%)` | :arrow_up: |
| [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nUmVzcG9uc2VVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [172 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...3411145](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7523:
URL: https://github.com/apache/pinot/pull/7523
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a174f2e) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `41.73%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
=============================================
- Coverage 70.75% 29.02% -41.74%
=============================================
Files 1523 1514 -9
Lines 75540 75325 -215
Branches 11007 10999 -8
=============================================
- Hits 53452 21860 -31592
- Misses 18422 51477 +33055
+ Partials 3666 1988 -1678
```
| Flag | Coverage Δ | |
|---|---|---|
| integration2 | `29.02% <0.00%> (-0.05%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (-66.14%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/QuotaConfig.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1F1b3RhQ29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1021 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...a174f2e](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9933aba) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `40.11%`.
> The diff coverage is `40.00%`.
> :exclamation: Current head 9933aba differs from pull request most recent head ca33f55. Consider uploading reports for the commit ca33f55 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
=============================================
- Coverage 70.75% 30.64% -40.12%
=============================================
Files 1523 1514 -9
Lines 75540 75294 -246
Branches 11007 10996 -11
=============================================
- Hits 53452 23072 -30380
- Misses 18422 50168 +31746
+ Partials 3666 2054 -1612
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `30.64% <40.00%> (?)` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ent/index/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `0.00% <0.00%> (-82.97%)` | :arrow_down: |
| [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `0.00% <0.00%> (-74.08%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (-66.14%)` | :arrow_down: |
| [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `58.26% <16.66%> (-24.28%)` | :arrow_down: |
| [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | `75.45% <62.85%> (-17.54%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==) | `80.24% <100.00%> (+2.60%)` | :arrow_up: |
| [.../controller/api/ControllerAdminApiApplication.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvQ29udHJvbGxlckFkbWluQXBpQXBwbGljYXRpb24uamF2YQ==) | `92.45% <100.00%> (+0.29%)` | :arrow_up: |
| [...troller/helix/core/minion/ClusterInfoAccessor.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9DbHVzdGVySW5mb0FjY2Vzc29yLmphdmE=) | `75.86% <100.00%> (+15.86%)` | :arrow_up: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `28.35% <100.00%> (+5.74%)` | :arrow_up: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1092 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...ca33f55](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (196e6c3) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `8.40%`.
> The diff coverage is `21.42%`.
> :exclamation: Current head 196e6c3 differs from pull request most recent head 47f17eb. Consider uploading reports for the commit 47f17eb to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
============================================
- Coverage 70.75% 62.35% -8.41%
+ Complexity 3400 3325 -75
============================================
Files 1523 1514 -9
Lines 75540 75284 -256
Branches 11007 10994 -13
============================================
- Hits 53452 46941 -6511
- Misses 18422 24948 +6526
+ Partials 3666 3395 -271
```
| Flag | Coverage Δ | |
|---|---|---|
| integration2 | `29.05% <5.55%> (-0.01%)` | :arrow_down: |
| unittests1 | `69.82% <42.00%> (-0.02%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==) | `77.16% <0.00%> (-0.48%)` | :arrow_down: |
| [...ent/index/column/PhysicalColumnIndexContainer.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `82.96% <0.00%> (ø)` | |
| [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | `2.16% <2.85%> (-90.83%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `63.91% <4.34%> (-2.22%)` | :arrow_down: |
| [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `81.10% <16.66%> (-1.44%)` | :arrow_down: |
| [...troller/helix/core/minion/ClusterInfoAccessor.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9DbHVzdGVySW5mb0FjY2Vzc29yLmphdmE=) | `44.82% <60.00%> (-15.18%)` | :arrow_down: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `22.22% <100.00%> (-0.39%)` | :arrow_down: |
| [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.08% <100.00%> (+7.00%)` | :arrow_up: |
| [...pinot/controller/recommender/io/ConfigManager.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9Db25maWdNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...troller/recommender/io/metadata/FieldMetadata.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9tZXRhZGF0YS9GaWVsZE1ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [206 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...47f17eb](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#discussion_r722510986
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -303,17 +307,40 @@ public static void validateIngestionConfig(TableConfig tableConfig, @Nullable Sc
}
}
- private static void validateTaskConfigs(TableConfig tableConfig) {
+ private 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);
+ // TODO validate task config directly from PinotTaskGenerator API
+ if (taskConfig != null) {
+ if (taskConfig.isTaskTypeEnabled(SEGMENT_GENERATION_AND_PUSH_TASK_TYPE)) {
Review comment:
Not introduced in this PR, but `"schedule"` field applies to all the tasks, and we should check it for all the tasks by looping over the `taskConfig.getTaskTypeConfigsMap()`
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -303,17 +307,40 @@ public static void validateIngestionConfig(TableConfig tableConfig, @Nullable Sc
}
}
- private static void validateTaskConfigs(TableConfig tableConfig) {
+ private 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);
+ // TODO validate task config directly from PinotTaskGenerator API
+ if (taskConfig != null) {
+ if (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.isTaskTypeEnabled(REALTIME_TO_OFFLINE_TASK_TYPE)) {
+ Map<String, String> taskTypeConfig = taskConfig.getConfigsForTaskType(REALTIME_TO_OFFLINE_TASK_TYPE);
+ if (taskTypeConfig != null) {
Review comment:
The `null` check is redundant
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7523:
URL: https://github.com/apache/pinot/pull/7523
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a change in pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #7523: add validation for realtimeToOffline task
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7523:
URL: https://github.com/apache/pinot/pull/7523#issuecomment-934619746
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7523](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47f17eb) into [master](https://codecov.io/gh/apache/pinot/commit/c9e36dd0a53370d7b8dc24459426dc77497eb8ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9e36dd) will **decrease** coverage by `55.49%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7523 +/- ##
=============================================
- Coverage 70.75% 15.26% -55.50%
+ Complexity 3400 80 -3320
=============================================
Files 1523 1477 -46
Lines 75540 73781 -1759
Branches 11007 10832 -175
=============================================
- Hits 53452 11259 -42193
- Misses 18422 61711 +43289
+ Partials 3666 811 -2855
```
| Flag | Coverage Δ | |
|---|---|---|
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.26% <0.00%> (+0.74%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (-66.14%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/core/data/table/BaseTable.java](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0Jhc2VUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1195 more](https://codecov.io/gh/apache/pinot/pull/7523/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c9e36dd...47f17eb](https://codecov.io/gh/apache/pinot/pull/7523?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org