You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/05/12 14:27:42 UTC

[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #15503: [enh] Issue 15455: Pulsar Admin: create subscripion with Properties (PIP-105)

nicoloboschi commented on code in PR #15503:
URL: https://github.com/apache/pulsar/pull/15503#discussion_r871438000


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentSubscription.java:
##########
@@ -518,4 +521,8 @@ public long getLastActive() {
     public void updateLastActive() {
         this.lastActive = System.currentTimeMillis();
     }
+
+    public Map<String, String> getSubscriptionProperties() {
+        return subscriptionProperties;

Review Comment:
   better to return an immutable map here



##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdPersistentTopics.java:
##########
@@ -507,8 +512,23 @@ void run() throws PulsarAdminException {
             } else {
                 messageId = validateMessageIdString(messageIdStr);
             }
-
-            getPersistentTopics().createSubscription(persistentTopic, subscriptionName, messageId);
+            Map<String, String> map = new HashMap<>();
+            if (properties != null) {
+                for (String property : properties) {
+                    if (!property.contains("=")) {

Review Comment:
   this condition is useless since if `=` is absent, the keyValue length will be 1



##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java:
##########
@@ -959,8 +962,23 @@ void run() throws PulsarAdminException {
             } else {
                 messageId = validateMessageIdString(messageIdStr);
             }
-
-            getTopics().createSubscription(topic, subscriptionName, messageId, replicated);
+            Map<String, String> map = new HashMap<>();
+            if (properties != null) {
+                for (String property : properties) {
+                    if (!property.contains("=")) {

Review Comment:
   this condition is useless since if = is absent, the keyValue length will be 1



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org