frankgh commented on code in PR #198: URL: https://github.com/apache/cassandra-sidecar/pull/198#discussion_r1987628220
########## server/src/main/java/org/apache/cassandra/sidecar/routes/ReportSchemaHandler.java: ########## @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.sidecar.routes; + +import java.util.Collections; +import java.util.Set; + +import com.datastax.driver.core.Metadata; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.core.net.SocketAddress; +import io.vertx.ext.auth.authorization.Authorization; +import io.vertx.ext.web.RoutingContext; +import org.apache.cassandra.sidecar.acl.authorization.BasicPermissions; +import org.apache.cassandra.sidecar.concurrent.ExecutorPools; +import org.apache.cassandra.sidecar.datahub.SchemaReporter; +import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * An implementation of {@link AbstractHandler} used to trigger an immediate, + * synchronous conversion and report of the current schema + */ +@Singleton +public class ReportSchemaHandler extends AbstractHandler<Void> implements AccessProtected +{ + @NotNull + private final SchemaReporter schemaReporter; + + /** + * Constructs a new instance of {@link ReportSchemaHandler} using the provided instances + * of {@link InstanceMetadataFetcher}, {@link ExecutorPools}, and {@link SchemaReporter} + * + * @param metadata the metadata fetcher + * @param executor executor pools for blocking executions + * @param reporter schema reporter to use for the conversion + */ + @Inject + public ReportSchemaHandler(@NotNull InstanceMetadataFetcher metadata, + @NotNull ExecutorPools executor, + @NotNull SchemaReporter reporter) + { + super(metadata, executor, null); + + schemaReporter = reporter; + } + + /** + * {@inheritDoc} + */ + @Override + @NotNull + public Set<Authorization> requiredAuthorizations() + { + return Collections.singleton(BasicPermissions.REPORT_SCHEMA.toAuthorization()); + } + + /** + * {@inheritDoc} + */ + @Override + @Nullable + protected Void extractParamsOrThrow(@NotNull RoutingContext context) + { + return null; + } + + /** + * {@inheritDoc} + */ + @Override + protected void handleInternal(@NotNull RoutingContext context, + @NotNull HttpServerRequest http, + @NotNull String host, + @NotNull SocketAddress address, + @Nullable Void request) + { + Metadata metadata = metadataFetcher.callOnFirstAvailableInstance(instance -> instance.delegate().metadata()); + + executorPools.service() + .runBlocking(() -> schemaReporter.process(metadata)) + .onSuccess(ignored -> context.end()) Review Comment: can we respond with a JSON response here? The expectation is that all endpoints return a JSON payload (except for streaming endpoints). I think we can do OK_STATUS ```suggestion .onSuccess(ignored -> context.json(OK_STATUS)) ``` ########## CHANGES.txt: ########## Review Comment: Looks like there is a merge conflict here. ########## server/src/test/java/org/apache/cassandra/sidecar/utils/InstanceMetadataFetcherTest.java: ########## Review Comment: this entire class should be reverted before merge. Changes are unrelated to the PR. ########## client/src/main/java/org/apache/cassandra/sidecar/client/SidecarClient.java: ########## @@ -567,6 +566,28 @@ public void streamCdcSegments(SidecarInstance sidecarInstance, .build(), streamConsumer); } + /** + * Sends a request to trigger an immediate, synchronous schema + * conversion and report on the specified instance of the Sidecar + * regardless of the periodic task schedule or status + * + * @param instance the {@link SidecarInstance} to receive the request + * @return a {@link CompletableFuture} for the request + */ + public CompletableFuture<Void> reportSchema(SidecarInstance instance) + { + // Create an instance of {@link RequestContext.Builder} using its + // constructor instead of the {@link this.requestBuilder()} method, + // since {@link NoRetryPolicy} is the preferred behavior here + + RequestContext context = new RequestContext.Builder() + .singleInstanceSelectionPolicy(instance) + .reportSchemaRequest() + .build(); + + return executor.executeRequestAsync(context); Review Comment: you can always override the policy like this: ```suggestion return executor.executeRequestAsync(requestBuilder() .singleInstanceSelectionPolicy(instance) .retryPolicy(new NoRetryPolicy()) .reportSchemaRequest() .build()); ``` ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java: ########## @@ -121,13 +121,16 @@ public final class ApiEndpointsV1 public static final String LIST_CDC_SEGMENTS_ROUTE = API_V1 + CDC_PATH + "/segments"; public static final String STREAM_CDC_SEGMENTS_ROUTE = LIST_CDC_SEGMENTS_ROUTE + "/" + SEGMENT_PATH_PARAM; + // Schema Reporting + private static final String REPORT_SCHEMA = "/report-schema"; + public static final String REPORT_SCHEMA_ROUTE = API_V1 + REPORT_SCHEMA; Review Comment: I'm not 100% convinced about this route. What do you think about this? ```suggestion public static final String REPORT_SCHEMA_ROUTE = API_V1 + "/services/schema-reporting/ REPORT_SCHEMA; ``` or something that scopes this feature to the schema reporting service. -- 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: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org
