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 2021/05/28 07:24:00 UTC

[GitHub] [pulsar] wangjialing218 opened a new pull request #10732: fix pulsar-admin functions update drops user-configs

wangjialing218 opened a new pull request #10732:
URL: https://github.com/apache/pulsar/pull/10732


   ### Motivation
   
   Fixes #10720
   
   ### Modifications
   
   Update UserConfig only when it's not empty.
   
   ### Verifying this change
   
   FunctionConfigUtilsTest.testMergeDifferentUserConfig()


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

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

[GitHub] [pulsar] wangjialing218 closed pull request #10732: fix pulsar-admin functions update drops user-configs

Posted by GitBox <gi...@apache.org>.
wangjialing218 closed pull request #10732:
URL: https://github.com/apache/pulsar/pull/10732


   


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

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

[GitHub] [pulsar] linlinnn commented on a change in pull request #10732: fix pulsar-admin functions update drops user-configs

Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10732:
URL: https://github.com/apache/pulsar/pull/10732#discussion_r641386080



##########
File path: pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java
##########
@@ -902,7 +902,7 @@ public static FunctionConfig validateUpdate(FunctionConfig existingConfig, Funct
         if (!StringUtils.isEmpty(newConfig.getOutput())) {
             mergedConfig.setOutput(newConfig.getOutput());
         }
-        if (newConfig.getUserConfig() != null) {
+        if (newConfig.getUserConfig() != null && !newConfig.getUserConfig().isEmpty()) {

Review comment:
       I close and reopen a similar [PR](https://github.com/apache/pulsar/pull/10731/files), would you mind taking a look?




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

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

[GitHub] [pulsar] linlinnn commented on a change in pull request #10732: fix pulsar-admin functions update drops user-configs

Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10732:
URL: https://github.com/apache/pulsar/pull/10732#discussion_r641384970



##########
File path: pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java
##########
@@ -902,7 +902,7 @@ public static FunctionConfig validateUpdate(FunctionConfig existingConfig, Funct
         if (!StringUtils.isEmpty(newConfig.getOutput())) {
             mergedConfig.setOutput(newConfig.getOutput());
         }
-        if (newConfig.getUserConfig() != null) {
+        if (newConfig.getUserConfig() != null && !newConfig.getUserConfig().isEmpty()) {

Review comment:
       in this way, we can not clear user configs.




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

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