Jackie-Jiang commented on a change in pull request #7921:
URL: https://github.com/apache/pinot/pull/7921#discussion_r774805609
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
##########
@@ -366,7 +374,7 @@ private void tuneConfig(TableConfig tableConfig, Schema
schema) {
TableConfigUtils.ensureStorageQuotaConstraints(tableConfig,
_controllerConf.getDimTableMaxSize());
}
- private String validateConfig(TableConfigs tableConfigs) {
+ private String validateConfig(TableConfigs tableConfigs, String typesToSkip)
{
Review comment:
(minor)
```suggestion
private String validateConfig(TableConfigs tableConfigs, @Nullable String
typesToSkip) {
```
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -508,22 +516,24 @@ public String checkTableConfig(String tableConfigStr) {
+ "Validate given table config and schema. If specified schema is
null, attempt to retrieve schema using the "
+ "table name. This API returns the table config that matches the
one you get from 'GET /tables/{tableName}'."
+ " This allows us to validate table config before apply.")
- public String validateTableAndSchema(TableAndSchemaConfig tableSchemaConfig)
{
+ public String validateTableAndSchema(
+ TableAndSchemaConfig tableSchemaConfig,
+ @ApiParam(value = "comma separated list of validation type(s) to skip.
supported types: (ALL|TASK)")
+ @QueryParam("validationTypesToSkip") String typesToSkip) {
TableConfig tableConfig = tableSchemaConfig.getTableConfig();
Schema schema = tableSchemaConfig.getSchema();
if (schema == null) {
schema = _pinotHelixResourceManager.getSchemaForTableConfig(tableConfig);
}
- return validateConfig(tableSchemaConfig.getTableConfig(), schema);
+ return validateConfig(tableSchemaConfig.getTableConfig(), typesToSkip,
schema);
}
- private String validateConfig(TableConfig tableConfig, Schema schema) {
+ private String validateConfig(TableConfig tableConfig, String typesToSkip,
Schema schema) {
Review comment:
(minor)
```suggestion
private String validateConfig(TableConfig tableConfig, Schema schema,
@Nullable String typesToSkip) {
```
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -153,7 +153,11 @@
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Adds a table", notes = "Adds a table")
- public SuccessResponse addTable(String tableConfigStr, @Context HttpHeaders
httpHeaders, @Context Request request) {
+ public SuccessResponse addTable(
+ String tableConfigStr,
+ @ApiParam(value = "comma separated list of validation type(s) to skip.
supported types: (ALL|TASK)")
Review comment:
Add `UPSERT` to the api description
```suggestion
@ApiParam(value = "comma separated list of validation type(s) to skip.
supported types: (ALL|TASK|UPSERT)")
```
Also annotate `typesToSkip` as nullable
Same for other places
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -90,20 +100,34 @@ private TableConfigUtils() {
*
* TODO: Add more validations for each section (e.g. validate conditions are
met for aggregateMetrics)
*/
- public static void validate(TableConfig tableConfig, @Nullable Schema
schema) {
+ public static void validate(TableConfig tableConfig, @Nullable Schema
schema, @Nullable String typesToSkip) {
+ List<ValidationType> skipTypes = parseTypesToSkipString(typesToSkip);
if (tableConfig.getTableType() == TableType.REALTIME) {
Preconditions.checkState(schema != null, "Schema should not be null for
REALTIME table");
}
// Sanitize the table config before validation
sanitize(tableConfig);
- validateValidationConfig(tableConfig, schema);
- validateIngestionConfig(tableConfig, schema);
- validateTierConfigList(tableConfig.getTierConfigsList());
- validateIndexingConfig(tableConfig.getIndexingConfig(), schema);
- validateFieldConfigList(tableConfig.getFieldConfigList(),
tableConfig.getIndexingConfig(), schema);
- validateUpsertConfig(tableConfig, schema);
- validatePartialUpsertStrategies(tableConfig, schema);
- validateTaskConfigs(tableConfig, schema);
+ // skip all validation if skip type ALL is selected.
+ if (!skipTypes.contains(ValidationType.ALL)) {
+ validateValidationConfig(tableConfig, schema);
+ validateIngestionConfig(tableConfig, schema);
+ validateTierConfigList(tableConfig.getTierConfigsList());
+ validateIndexingConfig(tableConfig.getIndexingConfig(), schema);
+ validateFieldConfigList(tableConfig.getFieldConfigList(),
tableConfig.getIndexingConfig(), schema);
+ if (!skipTypes.contains(ValidationType.UPSERT)) {
+ validateUpsertConfig(tableConfig, schema);
+ validatePartialUpsertStrategies(tableConfig, schema);
+ }
+ if (!skipTypes.contains(ValidationType.TASK)) {
+ validateTaskConfigs(tableConfig, schema);
+ }
+ }
+ }
+
+ private static List<ValidationType> parseTypesToSkipString(String
typesToSkip) {
Review comment:
Returns a set, and annotate with nullable
```suggestion
private static Set<ValidationType> parseTypesToSkipString(@Nullable String
typesToSkip) {
```
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -90,20 +100,34 @@ private TableConfigUtils() {
*
* TODO: Add more validations for each section (e.g. validate conditions are
met for aggregateMetrics)
*/
- public static void validate(TableConfig tableConfig, @Nullable Schema
schema) {
+ public static void validate(TableConfig tableConfig, @Nullable Schema
schema, @Nullable String typesToSkip) {
+ List<ValidationType> skipTypes = parseTypesToSkipString(typesToSkip);
if (tableConfig.getTableType() == TableType.REALTIME) {
Preconditions.checkState(schema != null, "Schema should not be null for
REALTIME table");
}
// Sanitize the table config before validation
sanitize(tableConfig);
- validateValidationConfig(tableConfig, schema);
- validateIngestionConfig(tableConfig, schema);
- validateTierConfigList(tableConfig.getTierConfigsList());
- validateIndexingConfig(tableConfig.getIndexingConfig(), schema);
- validateFieldConfigList(tableConfig.getFieldConfigList(),
tableConfig.getIndexingConfig(), schema);
- validateUpsertConfig(tableConfig, schema);
- validatePartialUpsertStrategies(tableConfig, schema);
- validateTaskConfigs(tableConfig, schema);
+ // skip all validation if skip type ALL is selected.
+ if (!skipTypes.contains(ValidationType.ALL)) {
+ validateValidationConfig(tableConfig, schema);
+ validateIngestionConfig(tableConfig, schema);
+ validateTierConfigList(tableConfig.getTierConfigsList());
+ validateIndexingConfig(tableConfig.getIndexingConfig(), schema);
+ validateFieldConfigList(tableConfig.getFieldConfigList(),
tableConfig.getIndexingConfig(), schema);
+ if (!skipTypes.contains(ValidationType.UPSERT)) {
+ validateUpsertConfig(tableConfig, schema);
+ validatePartialUpsertStrategies(tableConfig, schema);
+ }
+ if (!skipTypes.contains(ValidationType.TASK)) {
+ validateTaskConfigs(tableConfig, schema);
+ }
+ }
+ }
+
+ private static List<ValidationType> parseTypesToSkipString(String
typesToSkip) {
+ return typesToSkip == null ? Collections.emptyList() :
Arrays.stream(typesToSkip.split(","))
Review comment:
Consider skipping the whitespace for split, and convert to uppercase
before calling `valueOf()`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
