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/09/29 00:51:30 UTC

[GitHub] [pulsar] mattisonchao opened a new pull request, #17880: [improve][broker] Add URL check when add/update cluster.

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

   ### Motivation
   
   Currently, we do not do any URL checking when creating/updating clusters via the admin API. 
   
   ### Modifications
   
   - Add URL format check when creating/updating clusters.
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   
   ### Documentation
   - [x] `doc-not-needed` 
   (Please explain why)
   
   ### Matching PR in forked repository
   
   PR in forked repository:
   


-- 
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 commented on pull request #17880: [improve][broker] Add URL check when add/update cluster.

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

   @mattisonchao Please check the failed test.


-- 
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 commented on pull request #17880: [improve][broker] Add URL check when add/update cluster.

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

   @mattisonchao Please check the failed test.


-- 
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] mattisonchao commented on a diff in pull request #17880: [improve][broker] Add URL check when add/update cluster.

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


##########
pulsar-common/src/main/java/org/apache/pulsar/common/net/ServiceURI.java:
##########
@@ -106,7 +106,14 @@ public static ServiceURI create(String uriStr) {
 
         return create(uri);
     }
-
+    public static void validate(String urlStr) {

Review Comment:
   I see



-- 
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] mattisonchao commented on a diff in pull request #17880: [improve][broker] Add URL check when add/update cluster.

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


##########
pulsar-common/src/main/java/org/apache/pulsar/common/net/ServiceURI.java:
##########
@@ -106,7 +106,14 @@ public static ServiceURI create(String uriStr) {
 
         return create(uri);
     }
-
+    public static void validate(String urlStr) {

Review Comment:
   I'm not sure if we have to enhance this method to check pulsar supported service name.
   - pulsar
   - pulsar+ssl
   - http
   - https



-- 
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 commented on a diff in pull request #17880: [improve][broker] Add URL check when add/update cluster.

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


##########
pulsar-common/src/main/java/org/apache/pulsar/common/net/ServiceURI.java:
##########
@@ -106,7 +106,14 @@ public static ServiceURI create(String uriStr) {
 
         return create(uri);
     }
-
+    public static void validate(String urlStr) {

Review Comment:
   I think we should
   
   The service URL can only start with pulsar or pulsar+ssl
   The web service URL can only start with http or https



-- 
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] mattisonchao closed pull request #17880: [improve][broker] Add URL check when add/update cluster.

Posted by GitBox <gi...@apache.org>.
mattisonchao closed pull request #17880: [improve][broker] Add URL check when add/update cluster.
URL: https://github.com/apache/pulsar/pull/17880


-- 
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] github-actions[bot] commented on pull request #17880: [improve][broker] Add URL check when add/update cluster.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17880:
URL: https://github.com/apache/pulsar/pull/17880#issuecomment-1312619040

   The pr had no activity for 30 days, mark with Stale label.


-- 
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] mattisonchao commented on pull request #17880: [improve][broker] Add URL check when add/update cluster.

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

   Open another PR to do that.


-- 
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] codecov-commenter commented on pull request #17880: [improve][broker] Add URL check when add/update cluster.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #17880:
URL: https://github.com/apache/pulsar/pull/17880#issuecomment-1277015553

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/17880?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > :exclamation: No coverage uploaded for pull request base (`master@0678b82`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/17880/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/17880?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff            @@
   ##             master   #17880   +/-   ##
   =========================================
     Coverage          ?   26.08%           
     Complexity        ?     3221           
   =========================================
     Files             ?      367           
     Lines             ?    41172           
     Branches          ?     4217           
   =========================================
     Hits              ?    10740           
     Misses            ?    28682           
     Partials          ?     1750           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `26.08% <0.00%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   


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