+1 Regards Jiwei Guo (Tboy)
On Tue, Mar 29, 2022 at 4:16 PM Enrico Olivelli <eolive...@gmail.com> wrote: > +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 > > > >
