ankitsultana commented on code in PR #16677:
URL: https://github.com/apache/pinot/pull/16677#discussion_r2310861425
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -478,12 +452,171 @@ public String validateConfig(String tableConfigsStr,
return response.toString();
}
+ /**
+ * Previews the processed {@link TableConfigs} by applying validation,
defaults, and tuning
+ * without actually creating the table. Returns the final TableConfigs that
would be created.
+ */
+ @POST
+ @Path("/tables/preview")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Preview the processed TableConfigs",
+ notes = "Preview the processed TableConfigs with defaults applied and
tuning done, without creating the table")
+ @ManualAuthorization // performed after parsing TableConfigs
+ public String previewTableConfigs(String tableConfigsStr,
+ @ApiParam(value = "comma separated list of validation type(s) to skip.
supported types: (ALL|TASK|UPSERT)")
+ @QueryParam("validationTypesToSkip") @Nullable String typesToSkip,
+ @Context HttpHeaders httpHeaders, @Context Request request) {
+ try {
+ String databaseName =
DatabaseUtils.extractDatabaseFromHttpHeaders(httpHeaders);
+ Pair<TableConfigs, Map<String, Object>> tableConfigsAndUnrecognizedProps
=
+ processTableConfigs(tableConfigsStr, databaseName, typesToSkip);
+
+ TableConfigs tableConfigs = tableConfigsAndUnrecognizedProps.getLeft();
+ String rawTableName = tableConfigs.getTableName();
+
+ // validate permission for CREATE operation
+ String endpointUrl = request.getRequestURL().toString();
+ AccessControl accessControl = _accessControlFactory.create();
+ AccessControlUtils.validatePermission(rawTableName, AccessType.CREATE,
httpHeaders, endpointUrl, accessControl);
+ if (!accessControl.hasAccess(httpHeaders, TargetType.TABLE,
rawTableName, Actions.Table.CREATE_TABLE)) {
+ throw new ControllerApplicationException(LOGGER, "Permission denied",
Response.Status.FORBIDDEN);
+ }
+
+ ObjectNode response =
JsonUtils.objectToJsonNode(tableConfigs).deepCopy();
+ response.set("unrecognizedProperties",
JsonUtils.objectToJsonNode(tableConfigsAndUnrecognizedProps.getRight()));
+ return response.toString();
+ } catch (ControllerApplicationException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new ControllerApplicationException(LOGGER,
+ String.format("Failed to preview TableConfigs: %s", e.getMessage()),
+ Response.Status.INTERNAL_SERVER_ERROR, e);
+ }
+ }
+
+ /**
+ * Previews the processed {@link TableConfigs} for update operations by
applying validation, defaults, and tuning
+ * without actually updating the table. Returns the final TableConfigs that
would be updated.
+ */
+ @PUT
+ @Path("/tables/{tableName}/preview")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Preview the processed TableConfigs for update",
+ notes = "Preview the processed TableConfigs for update with defaults
applied and tuning done, without updating")
+ @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action =
Actions.Table.UPDATE_TABLE_CONFIGS)
+ @Authenticate(AccessType.READ)
+ public String previewUpdateTableConfigs(
+ @ApiParam(value = "TableConfigs name i.e. raw table name", required =
true) @PathParam("tableName")
+ String tableName,
+ @ApiParam(value = "comma separated list of validation type(s) to skip.
supported types: (ALL|TASK|UPSERT)")
+ @QueryParam("validationTypesToSkip") @Nullable String typesToSkip,
+ String tableConfigsStr, @Context HttpHeaders headers) {
+ try {
+ String databaseName =
DatabaseUtils.extractDatabaseFromHttpHeaders(headers);
+ tableName = DatabaseUtils.translateTableName(tableName, databaseName);
+
+ if (!_pinotHelixResourceManager.hasOfflineTable(tableName) &&
!_pinotHelixResourceManager.hasRealtimeTable(
+ tableName)) {
+ throw new ControllerApplicationException(LOGGER,
+ String.format("TableConfigs: %s does not exist. Use POST to create
it first.", tableName),
+ Response.Status.BAD_REQUEST);
+ }
+
+ Pair<TableConfigs, Map<String, Object>> tableConfigsAndUnrecognizedProps
=
+ processTableConfigsForUpdate(tableConfigsStr, tableName,
databaseName, typesToSkip);
+
+ TableConfigs tableConfigs = tableConfigsAndUnrecognizedProps.getLeft();
+
+ ObjectNode response =
JsonUtils.objectToJsonNode(tableConfigs).deepCopy();
+ response.set("unrecognizedProperties",
JsonUtils.objectToJsonNode(tableConfigsAndUnrecognizedProps.getRight()));
+ return response.toString();
+ } catch (ControllerApplicationException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new ControllerApplicationException(LOGGER,
+ String.format("Failed to preview update TableConfigs for: %s, %s",
tableName, e.getMessage()),
+ Response.Status.INTERNAL_SERVER_ERROR, e);
+ }
+ }
+
private void tuneConfig(TableConfig tableConfig, Schema schema) {
TableConfigTunerUtils.applyTunerConfigs(_pinotHelixResourceManager,
tableConfig, schema, Collections.emptyMap());
TableConfigUtils.ensureMinReplicas(tableConfig,
_controllerConf.getDefaultTableMinReplicas());
TableConfigUtils.ensureStorageQuotaConstraints(tableConfig,
_controllerConf.getDimTableMaxSize());
}
+ /**
+ * Processes a TableConfigs JSON string by parsing, validating, and applying
tuning.
+ * This shared method is used by both create and preview endpoints.
+ */
+ private Pair<TableConfigs, Map<String, Object>> processTableConfigs(String
tableConfigsStr,
Review Comment:
There's quite a lot of code duplication here.
Even with unit-tests I feel it's hard to guarantee long term that the
implementations of the update and preview APIs are not going to diverge
(reviewer and developers will have to know the context and ensure the logic is
consistent).
Can you try to create re-usable small methods that are used everywhere?
--
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]
