npawar commented on a change in pull request #4103: Add validations to table
config before updating a table config
URL: https://github.com/apache/incubator-pinot/pull/4103#discussion_r274713088
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -302,7 +302,7 @@ public SuccessResponse updateTableConfig(
ensureMinReplicas(tableConfig);
verifyTableConfigs(tableConfig);
- _pinotHelixResourceManager.setExistingTableConfig(tableConfig,
tableNameWithType, tableType);
+ _pinotHelixResourceManager.updateTableConfig(tableConfig,
tableNameWithType, tableType);
Review comment:
verifyTableCOnfigs compares the offline and realtime table configs for
hybrid tables.
validateTableTenantCOnfig is specifically for validating tenant configs.
I intentionally pushed the validation inside the update method, because
update and add are called from multiple files (v1 and v2 configs). And it was
better to have the call to validate in 1 place instead of in every endpoint.
Maybe the ensureMinReplicas and verify can also be pushed down. But that
would need more evaluation, because all tests use the add and update and
setExisting methods, which do not want all the validations
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
