[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649266#comment-16649266
 ] 

ASF GitHub Bot commented on SCB-928:


yhs0092 commented on a change in pull request #952: [SCB-928] support 
"collection-format" feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#discussion_r224979939
 
 

 ##
 File path: 
swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/converter/property/SwaggerParamCollectionFormat.java
 ##
 @@ -57,6 +68,41 @@ public String getSeparator() {
 if (null == rawParam) {
   return new String[0];
 }
-return rawParam.split(separator);
+return rawParam.split(separator, -1);
+  }
+
+  /**
+   * Join params with {@link #separator}.
+   * Null element will be ignored since {@code null} cannot be described in 
query array param.
+   *
+   * @return joined params, or return {@code null} if {@code params} is null 
or all elements of {@code params} are null.
+   */
+  public String joinParam(Collection params) {
+if (null == params) {
 
 Review comment:
   Here is my test code in `SwaggerParamCollectionFormat`: 
[SwaggerParamCollectionFormat.java](https://github.com/yhs0092/ServiceComb-Java-Chassis/blob/query_arr_param_joinParam/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/converter/property/SwaggerParamCollectionFormat.java)
   There is four tested method, `joinParam` is the original implementation, 
`joinParam2`,`joinParam3`,`joinParam4` is the implementation that take use of 
`StringUtils#join`.
   Performance test case is the `main` method in 
[SwaggerParamCollectionFormatTest.java](https://github.com/yhs0092/ServiceComb-Java-Chassis/blob/query_arr_param_joinParam/swagger/swagger-generator/generator-core/src/test/java/org/apache/servicecomb/swagger/converter/property/SwaggerParamCollectionFormatTest.java).
   The average result of these four method is respectively
   712/777/1609/1211 (test data is {"a","b","c"}),
   787/1528/1784/1489 (test data is {null, "a", null, "b", null, "", null, 
null, "c", null})
   we can find the original implementation is always the best, especially in 
round2, in which case there are null elements in query array.
   Therefore, I think we can keep this implementation since it's function meets 
our need.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649263#comment-16649263
 ] 

ASF GitHub Bot commented on SCB-928:


yhs0092 commented on a change in pull request #952: [SCB-928] support 
"collection-format" feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#discussion_r224979917
 
 

 ##
 File path: 
swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/converter/property/SwaggerParamCollectionFormat.java
 ##
 @@ -57,6 +68,41 @@ public String getSeparator() {
 if (null == rawParam) {
   return new String[0];
 }
-return rawParam.split(separator);
+return rawParam.split(separator, -1);
+  }
+
+  /**
+   * Join params with {@link #separator}.
+   * Null element will be ignored since {@code null} cannot be described in 
query array param.
+   *
+   * @return joined params, or return {@code null} if {@code params} is null 
or all elements of {@code params} are null.
+   */
+  public String joinParam(Collection params) {
+if (null == params) {
 
 Review comment:
   I've test `org.apache.commons.lang3.StringUtils#join(java.lang.Iterable, 
char)`, but there is a problem.
   Since `StringUtils#join` will treat null element as blank string `""`, it 
may cause some query param problem. If user pass a query array containing null 
element like `new String[]{"a",null,"b"}`, provider will get `new 
String[]{"a","","b"}` instead of `new String[]{"a","b"}`.
   If we add some extra logic to filter null element, the performance will be 
not so good as this implementation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649261#comment-16649261
 ] 

ASF GitHub Bot commented on SCB-928:


coveralls edited a comment on issue #952: [SCB-928] support "collection-format" 
feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#issuecomment-429520565
 
 
   
   [![Coverage 
Status](https://coveralls.io/builds/19508955/badge)](https://coveralls.io/builds/19508955)
   
   Coverage increased (+0.09%) to 86.321% when pulling 
**b30c51e78a0ee395f3030023c54ec7c78b216a03 on yhs0092:query_arr_param** into 
**ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649257#comment-16649257
 ] 

ASF GitHub Bot commented on SCB-928:


yhs0092 commented on a change in pull request #952: [SCB-928] support 
"collection-format" feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#discussion_r224979603
 
 

 ##
 File path: 
integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DownloadSchema.java
 ##
 @@ -183,7 +185,7 @@ public String getFilename() {
   @ApiResponse(code = 200, response = File.class, message = ""),
   })
   public ResponseEntity netInputStream(String content) throws 
IOException {
-URL url = new URL("http://localhost:9000/download/netInputStream?content=;
+URL url = new URL("http://localhost:; + netStreamPort + 
"/download/netInputStream?content="
 
 Review comment:
   This commit has been removed. :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-837) make http2 production ready

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649252#comment-16649252
 ] 

ASF GitHub Bot commented on SCB-837:


coveralls edited a comment on issue #947:  [SCB-837] make http2 production ready
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#issuecomment-428578957
 
 
   
   [![Coverage 
Status](https://coveralls.io/builds/19508805/badge)](https://coveralls.io/builds/19508805)
   
   Coverage increased (+0.007%) to 86.235% when pulling 
**8d6462e676671a87f07101adb258a54205dfd05b on heyile:metrics_http2** into 
**ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
>  Issue Type: Improvement
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client 
> performance is so bad.
>  
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it 
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent 
> stream count
>  
> we must perform a performance test, that make sure got a good result, and 
> then set the setting to be our default setting.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-837) make http2 production ready

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649242#comment-16649242
 ] 

ASF GitHub Bot commented on SCB-837:


heyile commented on a change in pull request #947:  [SCB-837] make http2 
production ready
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224978733
 
 

 ##
 File path: 
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
 ##
 @@ -38,7 +38,7 @@ public static int getThreadCount() {
   }
 
   public static int getHttp2ConnectionMaxPoolSize() {
-return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
 3)
+return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
 1)
 
 Review comment:
   ok, I will change it soon. (没往这里想都)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
>  Issue Type: Improvement
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client 
> performance is so bad.
>  
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it 
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent 
> stream count
>  
> we must perform a performance test, that make sure got a good result, and 
> then set the setting to be our default setting.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-837) make http2 production ready

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16649216#comment-16649216
 ] 

ASF GitHub Bot commented on SCB-837:


wujimin commented on a change in pull request #947:  [SCB-837] make http2 
production ready
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/947#discussion_r224976892
 
 

 ##
 File path: 
transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
 ##
 @@ -38,7 +38,7 @@ public static int getThreadCount() {
   }
 
   public static int getHttp2ConnectionMaxPoolSize() {
-return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
 3)
+return 
DynamicPropertyFactory.getInstance().getIntProperty("servicecomb.rest.client.http2.maxPoolSize",
 1)
 
 Review comment:
   why not just use vertx defined const value?
   eg: io.vertx.core.http.HttpClientOptions#DEFAULT_HTTP2_MAX_POOL_SIZE


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> make http2 production ready
> ---
>
> Key: SCB-837
> URL: https://issues.apache.org/jira/browse/SCB-837
> Project: Apache ServiceComb
>  Issue Type: Improvement
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: 何一乐
>Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> currenty, http2 client use all http1.1 settings, that cause http2 client 
> performance is so bad.
>  
> we need to conside http2 client settings at least:
> 1.concurrent stream in one connection, default value is 3, we must make it 
> bigger
> 2.maxPoolSize, http1.1 need a big pool, but http2 need a big concurrent 
> stream count
>  
> we must perform a performance test, that make sure got a good result, and 
> then set the setting to be our default setting.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (SCB-960) when consumer local failed(eg: LB failed), CompletableFuture callback can not get InvocationContext

2018-10-13 Thread wujimin (JIRA)
wujimin created SCB-960:
---

 Summary: when consumer local failed(eg: LB failed), 
CompletableFuture callback can not get InvocationContext
 Key: SCB-960
 URL: https://issues.apache.org/jira/browse/SCB-960
 Project: Apache ServiceComb
  Issue Type: Bug
  Components: Java-Chassis
Reporter: wujimin
Assignee: wujimin






--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16648951#comment-16648951
 ] 

ASF GitHub Bot commented on SCB-928:


wujimin commented on a change in pull request #952: [SCB-928] support 
"collection-format" feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#discussion_r224960727
 
 

 ##
 File path: 
swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/converter/property/SwaggerParamCollectionFormat.java
 ##
 @@ -57,6 +68,41 @@ public String getSeparator() {
 if (null == rawParam) {
   return new String[0];
 }
-return rawParam.split(separator);
+return rawParam.split(separator, -1);
+  }
+
+  /**
+   * Join params with {@link #separator}.
+   * Null element will be ignored since {@code null} cannot be described in 
query array param.
+   *
+   * @return joined params, or return {@code null} if {@code params} is null 
or all elements of {@code params} are null.
+   */
+  public String joinParam(Collection params) {
+if (null == params) {
 
 Review comment:
   try org.apache.commons.lang3.StringUtils#join(java.lang.Iterable, char)?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16648944#comment-16648944
 ] 

ASF GitHub Bot commented on SCB-928:


wujimin commented on a change in pull request #952: [SCB-928] support 
"collection-format" feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#discussion_r224960509
 
 

 ##
 File path: 
integration-tests/it-producer/src/main/java/org/apache/servicecomb/it/schema/DownloadSchema.java
 ##
 @@ -183,7 +185,7 @@ public String getFilename() {
   @ApiResponse(code = 200, response = File.class, message = ""),
   })
   public ResponseEntity netInputStream(String content) throws 
IOException {
-URL url = new URL("http://localhost:9000/download/netInputStream?content=;
+URL url = new URL("http://localhost:; + netStreamPort + 
"/download/netInputStream?content="
 
 Review comment:
   conflict with 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/946/commits/31b9925a3c45c340489096ccf7d045e8cde755e5
   
   copy that content to this commit or remote this commit, :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16648802#comment-16648802
 ] 

ASF GitHub Bot commented on SCB-928:


coveralls commented on issue #952: [SCB-928] support "collection-format" 
feature on query param
URL: 
https://github.com/apache/incubator-servicecomb-java-chassis/pull/952#issuecomment-429520565
 
 
   
   [![Coverage 
Status](https://coveralls.io/builds/19502410/badge)](https://coveralls.io/builds/19502410)
   
   Coverage increased (+0.09%) to 86.321% when pulling 
**13c7553ed190caf9064554a4e1c8e2dfc1d41320 on yhs0092:query_arr_param** into 
**ddd17bf55c139189b9f2d635c137c99f940d6a26 on apache:master**.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (SCB-928) support swagger "collection-format" feature

2018-10-13 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/SCB-928?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16648795#comment-16648795
 ] 

ASF GitHub Bot commented on SCB-928:


yhs0092 opened a new pull request #952: [SCB-928] support "collection-format" 
feature on query param
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/952
 
 
   Follow this checklist to help us incorporate your contribution quickly and 
easily:
   
- [x] Make sure there is a [JIRA 
issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually 
before you start working on it).  Trivial changes like typos do not require a 
JIRA issue.  Your pull request should address just this issue, without pulling 
in other changes.
- [x] Each commit in the pull request should have a meaningful subject line 
and body.
- [x] Format the pull request title like `[SCB-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA 
issue.
- [x] Write a pull request description that is detailed enough to 
understand what the pull request does, how, and why.
- [x] Run `mvn clean install` to make sure basic checks pass. A more 
thorough check will be performed on your pull request automatically.
- [x] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   ---
   Let users can transport query array param according to "collection-format" 
specified in swagger schema.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> support swagger "collection-format" feature
> ---
>
> Key: SCB-928
> URL: https://issues.apache.org/jira/browse/SCB-928
> Project: Apache ServiceComb
>  Issue Type: Task
>  Components: Java-Chassis
>Reporter: wujimin
>Assignee: YaoHaishi
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)