heesung-sn commented on code in PR #21531:
URL: https://github.com/apache/pulsar/pull/21531#discussion_r1385633354
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java:
##########
@@ -826,12 +824,39 @@ public void asyncDelete(String name, DeleteLedgerCallback
callback, Object ctx)
}
@Override
+ @Deprecated
public void asyncDelete(String name,
CompletableFuture<ManagedLedgerConfig> mlConfigFuture,
DeleteLedgerCallback callback, Object ctx) {
- CompletableFuture<ManagedLedgerImpl> future = ledgers.get(name);
+ mlConfigFuture.thenAccept(managedLedgerConfig -> {
+ asyncDelete(name, managedLedgerConfig, new DeleteLedgerCallback() {
Review Comment:
can we just pass the input `callback`?
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java:
##########
@@ -826,12 +824,39 @@ public void asyncDelete(String name, DeleteLedgerCallback
callback, Object ctx)
}
@Override
+ @Deprecated
public void asyncDelete(String name,
CompletableFuture<ManagedLedgerConfig> mlConfigFuture,
DeleteLedgerCallback callback, Object ctx) {
- CompletableFuture<ManagedLedgerImpl> future = ledgers.get(name);
+ mlConfigFuture.thenAccept(managedLedgerConfig -> {
+ asyncDelete(name, managedLedgerConfig, new DeleteLedgerCallback() {
+ @Override
+ public void deleteLedgerComplete(Object ctx) {
+ callback.deleteLedgerComplete(ctx);
+ }
+
+ @Override
+ public void deleteLedgerFailed(ManagedLedgerException
exception, Object ctx) {
+ callback.deleteLedgerFailed(exception, ctx);
+ }
+ }, ctx);
+ }).exceptionally(ex -> {
+ final Throwable rc = FutureUtil.unwrapCompletionException(ex);
+ callback.deleteLedgerFailed(getManagedLedgerException(rc), ctx);
+ return null;
+ });
+ }
+
+ @Override
+ public void asyncDelete(@Nonnull String name, @Nonnull ManagedLedgerConfig
managedLedgerConfig,
+ @Nonnull DeleteLedgerCallback callback, @Nullable
Object ctx) {
+ Objects.requireNonNull(name);
+ Objects.requireNonNull(managedLedgerConfig);
+ Objects.requireNonNull(callback);
+
+ final CompletableFuture<ManagedLedgerImpl> future = ledgers.get(name);
if (future == null) {
// Managed ledger does not exist and we're not currently trying to
open it
- deleteManagedLedger(name, mlConfigFuture, callback, ctx);
+ deleteManagedLedger(name, managedLedgerConfig, callback, ctx);
Review Comment:
Why not keep using `mlConfig` var name?
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java:
##########
@@ -826,12 +824,39 @@ public void asyncDelete(String name, DeleteLedgerCallback
callback, Object ctx)
}
@Override
+ @Deprecated
public void asyncDelete(String name,
CompletableFuture<ManagedLedgerConfig> mlConfigFuture,
DeleteLedgerCallback callback, Object ctx) {
- CompletableFuture<ManagedLedgerImpl> future = ledgers.get(name);
+ mlConfigFuture.thenAccept(managedLedgerConfig -> {
+ asyncDelete(name, managedLedgerConfig, new DeleteLedgerCallback() {
+ @Override
+ public void deleteLedgerComplete(Object ctx) {
+ callback.deleteLedgerComplete(ctx);
+ }
+
+ @Override
+ public void deleteLedgerFailed(ManagedLedgerException
exception, Object ctx) {
+ callback.deleteLedgerFailed(exception, ctx);
+ }
+ }, ctx);
+ }).exceptionally(ex -> {
+ final Throwable rc = FutureUtil.unwrapCompletionException(ex);
+ callback.deleteLedgerFailed(getManagedLedgerException(rc), ctx);
+ return null;
+ });
+ }
+
+ @Override
+ public void asyncDelete(@Nonnull String name, @Nonnull ManagedLedgerConfig
managedLedgerConfig,
+ @Nonnull DeleteLedgerCallback callback, @Nullable
Object ctx) {
+ Objects.requireNonNull(name);
+ Objects.requireNonNull(managedLedgerConfig);
+ Objects.requireNonNull(callback);
Review Comment:
Do we have any coding guidelines for these null checks and `@Nonnull`
annotation?
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactory.java:
##########
@@ -183,9 +185,24 @@ void delete(String name,
CompletableFuture<ManagedLedgerConfig> mlConfigFuture)
* @throws InterruptedException
* @throws ManagedLedgerException
*/
+ @Deprecated
void asyncDelete(String name, CompletableFuture<ManagedLedgerConfig>
mlConfigFuture,
Review Comment:
What's our plan to remove this deprecated code? Can we clean the code to
make the caller use the new one?
--
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]
