eolivelli commented on a change in pull request #11494:
URL: https://github.com/apache/pulsar/pull/11494#discussion_r678910681



##########
File path: 
pulsar-broker/src/test/java/org/apache/pulsar/broker/transaction/TransactionTest.java
##########
@@ -167,4 +168,22 @@ public void testGetTxnID() throws Exception {
         Assert.assertEquals(txnID.getLeastSigBits(), 1);
         Assert.assertEquals(txnID.getMostSigBits(), 0);
     }
+
+    @Test
+    public void testSubscriptionRecreateTopic() throws PulsarAdminException, 
PulsarClientException {
+        String topicName = 
"persistent://pulsar/system/tests_SubscriptionRecreateTopic";
+
+        admin.topics().createNonPartitionedTopic(topicName);
+        admin.topics().unload(topicName);
+        try {
+            admin.topics().createNonPartitionedTopic(topicName);
+        } catch (Exception e){

Review comment:
       you can use catch (PulsarAdminException.ConflictException) and let other 
exceptions be not caught

##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/MLPendingAckStoreProvider.java
##########
@@ -51,7 +52,8 @@
         PersistentTopic originPersistentTopic = (PersistentTopic) 
subscription.getTopic();
         String pendingAckTopicName = MLPendingAckStore
                 
.getTransactionPendingAckStoreSuffix(originPersistentTopic.getName(), 
subscription.getName());
-
+        ManagedLedgerConfig managedLedgerConfig = 
originPersistentTopic.getManagedLedger().getConfig();
+        managedLedgerConfig.setCreateIfMissing(true);

Review comment:
       this is not good, we cannot alter the configuration of another component 
this way.
   
   if "createIfMissing" is required we have to set it upstream

##########
File path: 
pulsar-broker/src/test/java/org/apache/pulsar/broker/transaction/TransactionTest.java
##########
@@ -167,4 +168,22 @@ public void testGetTxnID() throws Exception {
         Assert.assertEquals(txnID.getLeastSigBits(), 1);
         Assert.assertEquals(txnID.getMostSigBits(), 0);
     }
+
+    @Test
+    public void testSubscriptionRecreateTopic() throws PulsarAdminException, 
PulsarClientException {
+        String topicName = 
"persistent://pulsar/system/tests_SubscriptionRecreateTopic";
+
+        admin.topics().createNonPartitionedTopic(topicName);
+        admin.topics().unload(topicName);
+        try {
+            admin.topics().createNonPartitionedTopic(topicName);

Review comment:
       please add a "fail()" after this line, other wise if 
`createNonPartitionedTopic` succeeds then the test will pass




-- 
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]


Reply via email to