+1 (binding) Enrico
Il giorno mar 29 mar 2022 alle ore 09:55 PengHui Li <peng...@apache.org> ha scritto: > > +1, > > Penghui > > On Fri, Mar 25, 2022 at 10:13 AM mattison chao <mattisonc...@gmail.com> > wrote: > > > This is the voting thread for PIP-149. It will stay open for at least 48 > > hours. > > > > https://github.com/apache/pulsar/issues/14365 > > > > Pasted below for quoting convenience. > > > > ----- > > > > Motivation > > > > The Rest API was originally designed to be implemented asynchronously, but > > with the iteration of functions, some synchronous implementations were > > added, resulting in many asynchronous methods called synchronous > > implementations. Also, many synchronous calls do not add timeouts. This > > greatly reduces concurrency, user operations, and experience. > > In order to prevent more problems, and improve code readability and > > maintainability, we intend to refactor these synchronous calls and > > standardize the implementation of the API. > > > > Related discussion: > > https://lists.apache.org/thread/pkkz2jgwtzpksp6d4rdm1pyxzb3z6vmg > > > > Goals > > > > Try to avoid synchronous method calls in asynchronous methods. > > Async variable (AsyncResponse) is placed in the first parameter position. > > Async variable (AsyncResponse) cannot be substituted into method > > implementations. > > Add more tests and increase the coverage. > > Modification > > Avoid synchronous method calls in asynchronous methods. > > > > protected void internalDeleteNamespace(boolean authoritative) { > > validateTenantOperation(namespaceName.getTenant(), > > TenantOperation.DELETE_NAMESPACE); > > validatePoliciesReadOnlyAccess(); > > } > > Suggest to do like this: > > > > protected CompletableFuture<Void> internalDeleteNamespace(boolean > > authoritative) { > > return validateTenantOperationAsync(namespaceName.getTenant(), > > TenantOperation.DELETE_NAMESPACE) > > .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync()); > > } > > Async variable (AsyncResponse) is placed in the first parameter position > > > > public void deleteNamespace(@Suspended final AsyncResponse asyncResponse, > > @PathParam("tenant") String tenant, > > @PathParam("namespace") String namespace, > > @QueryParam("force") @DefaultValue("false") boolean force, > > @QueryParam("authoritative") @DefaultValue("false") boolean > > authoritative) { > > > > Async variable (AsyncResponse) cannot be substituted into method > > implementations > > > > internalCreateNonPartitionedTopicAsync(asyncResponse, authoritative, > > properties); > > Suggest to do like this: > > > > internalCreateNonPartitionedTopicAsync(authoritative, properties) > > .thenAccept(__ -> > > asyncResponse.resume(Response.noContent().build())) > > .exceptionally(ex -> { > > resumeAsyncResponseExceptionally(asyncResponse, ex.getCause()); > > return null; > > }); > > > > Task tracking > > In order to unify the modification and track the modified part, it's better > > to open an issue to track, like #14353, #14013, #13854. > > > > --- > > Best, > > Mattison > >
