[jira] [Commented] (SCB-928) support swagger "collection-format" feature
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
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
[ 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
[ 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
[ 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
[ 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)