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/07/28 13:14:25 UTC

[GitHub] [pulsar] SignorMercurio opened a new pull request, #16853: [feature][doc] Show language support for pulsar-admin functions flags

SignorMercurio opened a new pull request, #16853:
URL: https://github.com/apache/pulsar/pull/16853

   Fixes #12381
   
   ### Motivation
   
   Please check out the original issue.
   
   ### Modifications
   
   Add `#language` tags in parameter descriptions in `CmdFunctions` so that `CmdGenerateDocument` can generate a new language support column.
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   ### Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - Anything that affects deployment: no
   
   ### Documentation
   
   Check the box below or label this PR directly.
   
   Need to update docs? 
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
     
   - [x] `doc-not-needed` 
   (Please explain why)
   
   Only adjusted existing generation command.
   
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)


-- 
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] Anonymitaet commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1200590408

   ping @urfreespace again


-- 
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] SignorMercurio commented on a diff in pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on code in PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#discussion_r933498215


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java:
##########
@@ -969,7 +990,7 @@ void runCmd() throws Exception {
     @Parameters(commandDescription = "Update a Pulsar Function that has been deployed to a Pulsar cluster")
     class UpdateFunction extends FunctionDetailsCommand {
 
-        @Parameter(names = "--update-auth-data", description = "Whether or not to update the auth data")
+        @Parameter(names = "--update-auth-data", description = "Whether or not to update the auth data #Java,Python")

Review Comment:
   Indeed, but AFAIK there's no way to add a new field here. Please see [JCommander Docs](https://jcommander.org) for more info.



-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1201223663

   /pulsarbot run-failure-checks


-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1200830847

   /pulsarbot run-failure-checks


-- 
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] SignorMercurio commented on a diff in pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on code in PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#discussion_r934095711


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java:
##########
@@ -185,40 +185,42 @@ void processArguments() throws Exception {
      */
     @Getter
     abstract class FunctionDetailsCommand extends BaseCommand {
-        @Parameter(names = "--fqfn", description = "The Fully Qualified Function Name (FQFN) for the function")
+        @Parameter(names = "--fqfn", description = "The Fully Qualified Function Name (FQFN) for the function"
+                + " #Java,Python")
         protected String fqfn;
-        @Parameter(names = "--tenant", description = "The tenant of a Pulsar Function")
+        @Parameter(names = "--tenant", description = "The tenant of a Pulsar Function #Java,Python,Go")

Review Comment:
   Okay, I've added the space now.



-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1199000674

   /pulsarbot run-failure-checks


-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1201002108

   /pulsarbot run-failure-checks


-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1198129725

   @Anonymitaet PTAL


-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1201971363

   /pulsarbot run-failure-checks


-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1201897305

   /pulsarbot run-failure-checks


-- 
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] tisonkun commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1202005647

   @Anonymitaet @urfreespace  all required tasks passed. Shall we merge this patch then?


-- 
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] tisonkun commented on a diff in pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#discussion_r933313065


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java:
##########
@@ -969,7 +990,7 @@ void runCmd() throws Exception {
     @Parameters(commandDescription = "Update a Pulsar Function that has been deployed to a Pulsar cluster")
     class UpdateFunction extends FunctionDetailsCommand {
 
-        @Parameter(names = "--update-auth-data", description = "Whether or not to update the auth data")
+        @Parameter(names = "--update-auth-data", description = "Whether or not to update the auth data #Java,Python")

Review Comment:
   Why not add a new parameter? Parsing string literals is generally error prone, though.



-- 
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] Anonymitaet commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1198799686

   @urfreespace 
   could you please review this PR from the technical perspective? Thank you!


-- 
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] urfreespace commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
urfreespace commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1200606615

   @SignorMercurio could you please provide a screenshot of the test result? thanks.


-- 
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] SignorMercurio commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
SignorMercurio commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1200613789

   ![image](https://user-images.githubusercontent.com/32540679/182060524-1225b0e4-9f9e-4822-8999-49e507af09f0.png)
   


-- 
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] Anonymitaet commented on a diff in pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#discussion_r934093488


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java:
##########
@@ -185,40 +185,42 @@ void processArguments() throws Exception {
      */
     @Getter
     abstract class FunctionDetailsCommand extends BaseCommand {
-        @Parameter(names = "--fqfn", description = "The Fully Qualified Function Name (FQFN) for the function")
+        @Parameter(names = "--fqfn", description = "The Fully Qualified Function Name (FQFN) for the function"
+                + " #Java,Python")
         protected String fqfn;
-        @Parameter(names = "--tenant", description = "The tenant of a Pulsar Function")
+        @Parameter(names = "--tenant", description = "The tenant of a Pulsar Function #Java,Python,Go")

Review Comment:
   ```suggestion
           @Parameter(names = "--tenant", description = "The tenant of a Pulsar Function #Java, Python, Go")
   ```
   Can you add a space after `comma`? That's the basic English rule 😀
   
   Screeshots LGTM



-- 
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] Anonymitaet merged pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
Anonymitaet merged PR #16853:
URL: https://github.com/apache/pulsar/pull/16853


-- 
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] tisonkun commented on a diff in pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#discussion_r933701212


##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java:
##########
@@ -969,7 +990,7 @@ void runCmd() throws Exception {
     @Parameters(commandDescription = "Update a Pulsar Function that has been deployed to a Pulsar cluster")
     class UpdateFunction extends FunctionDetailsCommand {
 
-        @Parameter(names = "--update-auth-data", description = "Whether or not to update the auth data")
+        @Parameter(names = "--update-auth-data", description = "Whether or not to update the auth data #Java,Python")

Review Comment:
   @SignorMercurio thanks for your information! It seems a JCommander limitation. Then I'm OK with current approach, while we can possibly use another annotation to avoid string parsing but it's not a blocker to this patch.



-- 
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] Anonymitaet commented on pull request #16853: [feature][doc] Show language support for pulsar-admin functions flags

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16853:
URL: https://github.com/apache/pulsar/pull/16853#issuecomment-1200591528

   @SignorMercurio can you show some preview screenshots? Want to check if it's the same as we discussed last time.


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