noob-se7en commented on code in PR #16675:
URL: https://github.com/apache/pinot/pull/16675#discussion_r2345001575
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -521,6 +532,39 @@ private void validateConfig(TableConfigs tableConfigs,
String database, @Nullabl
if (offlineTableConfig != null && realtimeTableConfig != null) {
TableConfigUtils.verifyHybridTableConfigs(rawTableName,
offlineTableConfig, realtimeTableConfig);
}
+
+ // Parse validation types to skip (following same pattern as
TableConfigUtils.validate)
+ Set<TableConfigUtils.ValidationType> skipTypes = typesToSkip == null ?
Collections.emptySet()
+ : Arrays.stream(typesToSkip.split(",")).map(s ->
TableConfigUtils.ValidationType.valueOf(s.toUpperCase()))
+ .collect(Collectors.toSet());
Review Comment:
Pls use the TableConfigsUtil method for this, we can make that public.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -210,7 +216,7 @@ public ConfigSuccessResponse addConfig(
}
TableConfigs tableConfigs = tableConfigsAndUnrecognizedProps.getLeft();
String databaseName =
DatabaseUtils.extractDatabaseFromHttpHeaders(httpHeaders);
- validateConfig(tableConfigs, databaseName, typesToSkip);
+ validateConfig(tableConfigs, databaseName, SKIP_CLUSTER_VALIDATIONS);
Review Comment:
+1
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -106,6 +109,9 @@ public class TableConfigsRestletResource {
public static final Logger LOGGER =
LoggerFactory.getLogger(TableConfigsRestletResource.class);
+ // Skip cluster validations during table creation/update since they're
performed in the actual table operations
+ private static final String SKIP_CLUSTER_VALIDATIONS =
"TENANT,MINION_INSTANCES,ACTIVE_TASKS";
Review Comment:
Pls update API param swagger documentation if including above more types.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -521,6 +532,39 @@ private void validateConfig(TableConfigs tableConfigs,
String database, @Nullabl
if (offlineTableConfig != null && realtimeTableConfig != null) {
TableConfigUtils.verifyHybridTableConfigs(rawTableName,
offlineTableConfig, realtimeTableConfig);
}
+
+ // Parse validation types to skip (following same pattern as
TableConfigUtils.validate)
+ Set<TableConfigUtils.ValidationType> skipTypes = typesToSkip == null ?
Collections.emptySet()
+ : Arrays.stream(typesToSkip.split(",")).map(s ->
TableConfigUtils.ValidationType.valueOf(s.toUpperCase()))
+ .collect(Collectors.toSet());
+
+ // Cluster-aware validations
+ if (!skipTypes.contains(TableConfigUtils.ValidationType.ALL)) {
Review Comment:
why not add these new validations in TableConfigUtils.validate method itself?
--
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]
