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/11/23 18:55:27 UTC

[GitHub] [pulsar] dlg99 opened a new pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

dlg99 opened a new pull request #12947:
URL: https://github.com/apache/pulsar/pull/12947


   ### Motivation
   
   When k8s runtime is being used the buitlin connectors get uploaded to BK and the uploaded version used later even after the pulsar upgrade. This means that after the upgrade one needs to delete and re-create the connectors.
   
   ### Modifications
   
   Added option to skip the upload to the BK and use the connector package from the connectors directory.
   
   ### Verifying this change
   
   Tested locally (minikube, k8s runtime).
   I don't see any unit/integration test that cover k8s runtime e2e.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
   Added new configuration parameter, default behavior stays the same.
   
     - Dependencies (does it add or upgrade a dependency): (yes / no)
     - The public API: (yes / no)
     - The schema: (yes / no / don't know)
     - The default values of configurations: (yes / no) - *new configuration parameter*
     - The wire protocol: (yes / no)
     - The rest endpoints: (yes / no)
     - The admin cli options: (yes / no)
     - Anything that affects deployment: (yes / no / don't know)
   
   ### Documentation
   
   Check the box below and label this PR (if you have committer privilege).
   
   Need to update docs? 
   
   - [x] `doc-required` 
     
     (If you need help on updating docs, create a doc issue)
     
   - [ ] `no-need-doc` 
     
     (Please explain why)
     
   - [ ] `doc` 
     
     (If this PR contains doc changes)
   
   
   


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



[GitHub] [pulsar] dlg99 commented on a change in pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

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



##########
File path: pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java
##########
@@ -179,6 +179,11 @@
         category = CATEGORY_FUNCTIONS,
         doc = "The path to the location to locate builtin functions"
     )
+    private Boolean uploadBuiltinSinksSources = true;

Review comment:
       with lombok it only matters if it generates accessor methods like "getBoolean" vs "isBoolean".




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



[GitHub] [pulsar] dlg99 commented on pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #12947:
URL: https://github.com/apache/pulsar/pull/12947#issuecomment-985932081


   @jerrypeng @nicoloboschi I added unit tests. Please take 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.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] dlg99 commented on pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #12947:
URL: https://github.com/apache/pulsar/pull/12947#issuecomment-982191037


   @merlimat  @jerrypeng can you take a look please?


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



[GitHub] [pulsar] codelipenghui merged pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #12947:
URL: https://github.com/apache/pulsar/pull/12947


   


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



[GitHub] [pulsar] nicoloboschi commented on a change in pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

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



##########
File path: pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java
##########
@@ -179,6 +179,11 @@
         category = CATEGORY_FUNCTIONS,
         doc = "The path to the location to locate builtin functions"
     )
+    private Boolean uploadBuiltinSinksSources = true;

Review comment:
       Ok but I think using 'is' better then 'get' for Java booleans, btw we can keep in this way, no problem




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



[GitHub] [pulsar] dlg99 commented on a change in pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

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



##########
File path: pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java
##########
@@ -1272,6 +1283,20 @@ private StreamingOutput getStreamingOutput(String pkgPath) {
                 URI url = URI.create(pkgPath);
                 File file = new File(url.getPath());
                 Files.copy(file.toPath(), output);
+            } else if(pkgPath.startsWith(Utils.BUILTIN) && !worker().getWorkerConfig().getUploadBuiltinSinksSources()) {

Review comment:
       fixed




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



[GitHub] [pulsar] nicoloboschi commented on a change in pull request #12947: Option to not upload builtin connectors to BK for k8s runtime

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



##########
File path: pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/rest/api/ComponentImpl.java
##########
@@ -1272,6 +1283,20 @@ private StreamingOutput getStreamingOutput(String pkgPath) {
                 URI url = URI.create(pkgPath);
                 File file = new File(url.getPath());
                 Files.copy(file.toPath(), output);
+            } else if(pkgPath.startsWith(Utils.BUILTIN) && !worker().getWorkerConfig().getUploadBuiltinSinksSources()) {

Review comment:
       can we add a unit test for this code block ?  

##########
File path: pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java
##########
@@ -179,6 +179,11 @@
         category = CATEGORY_FUNCTIONS,
         doc = "The path to the location to locate builtin functions"
     )
+    private Boolean uploadBuiltinSinksSources = true;

Review comment:
       why not `boolean` ? 




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