You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by "MichaelDeSteven (via GitHub)" <gi...@apache.org> on 2023/02/04 08:36:59 UTC

[GitHub] [shenyu] MichaelDeSteven opened a new pull request, #4352: [ISSUE #3450] add brpc integrated test

MichaelDeSteven opened a new pull request, #4352:
URL: https://github.com/apache/shenyu/pull/4352

   <!-- Describe your PR here; eg. Fixes #issueNo -->
   
   <!--
   Thank you for proposing a pull request. This template will guide you through the essential steps necessary for a pull request.
   -->
   Make sure that:
   
   - [ ] You have read the [contribution guidelines](https://shenyu.apache.org/community/contributor-guide).
   - [ ] You submit test cases (unit or integration tests) that back your changes.
   - [ ] Your local test passed `./mvnw clean install -Dmaven.javadoc.skip=true`.
   


-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103583578


##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/cache/ApplicationConfigCache.java:
##########
@@ -62,7 +62,16 @@ public final class ApplicationConfigCache {
 
     private JDKProxyFactory proxyFactory;
 
-    private final LoadingCache<String, AsyncGenericService> cache = CacheBuilder.newBuilder()
+    private final LoadingCache<String, ServiceConfig> serviceConfigCache = CacheBuilder.newBuilder()
+            .maximumSize(Constants.CACHE_MAX_COUNT)
+            .build(new CacheLoader<String, ServiceConfig>() {
+                @Override
+                public ServiceConfig load(@NonNull final String key) {
+                    return null;

Review Comment:
   need give a default `ServiceConfig `.



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103583578


##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/cache/ApplicationConfigCache.java:
##########
@@ -62,7 +62,16 @@ public final class ApplicationConfigCache {
 
     private JDKProxyFactory proxyFactory;
 
-    private final LoadingCache<String, AsyncGenericService> cache = CacheBuilder.newBuilder()
+    private final LoadingCache<String, ServiceConfig> serviceConfigCache = CacheBuilder.newBuilder()
+            .maximumSize(Constants.CACHE_MAX_COUNT)
+            .build(new CacheLoader<String, ServiceConfig>() {
+                @Override
+                public ServiceConfig load(@NonNull final String key) {
+                    return null;

Review Comment:
   need give a default `ServiceConfig`.



##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/cache/ApplicationConfigCache.java:
##########
@@ -116,9 +139,7 @@ public AsyncGenericService build(final MetaData metaData) {
         if (Objects.isNull(clientConfig)) {
             throw new UnsupportedOperationException("unsupport!!");
         }

Review Comment:
   When `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#init` is not called, try to initialize `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#initService` , `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#build` will throw `UnsupportedOperationException`.
   
   Therefore, we should delay the action of building `AsyncGenericService` until the `runtime`. Of course, caching `AsyncGenericService`  is recommended.



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103582061


##########
shenyu-integrated-test/shenyu-integrated-test-brpc/pom.xml:
##########
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one or more
+  ~ contributor license agreements.  See the NOTICE file distributed with
+  ~ this work for additional information regarding copyright ownership.
+  ~ The ASF licenses this file to You under the Apache License, Version 2.0
+  ~ (the "License"); you may not use this file except in compliance with
+  ~ the License.  You may obtain a copy of the License at
+  ~
+  ~     http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <parent>
+        <artifactId>shenyu-integrated-test</artifactId>
+        <groupId>org.apache.shenyu</groupId>
+        <version>2.5.1-SNAPSHOT</version>

Review Comment:
   fix version to `2.6.0-SNAPSHOT`



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang closed pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang closed pull request #4352: [ISSUE #3450] add brpc integrated test 
URL: https://github.com/apache/shenyu/pull/4352


-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103581381


##########
shenyu-integrated-test/shenyu-integrated-test-brpc/docker-compose.yml:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+version: "3.9"
+services:
+  shenyu-admin:
+    image: apache/shenyu-admin:latest
+    container_name: shenyu-admin
+    restart: always
+    networks:
+      - shenyu
+    ports:
+      - "9095:9095"
+    environment:
+      - SPRING_PROFILES_ACTIVE=h2
+      - shenyu.database.init_script=sql-script/h2/schema.sql
+    healthcheck:
+      test: ["CMD-SHELL", "wget -q -O - http://shenyu-admin:9095/actuator/health | grep UP || exit 1"]
+      timeout: 2s
+      retries: 30
+      start_period: 5s
+
+  shenyu-examples-brpc-service:
+    deploy:
+      resources:
+        limits:
+          memory: 2048M
+    container_name: shenyu-examples-brpc-service
+    image: shenyu-examples-brpc-service:latest
+    restart: always
+    environment:
+      - shenyu.register.serverLists=http://shenyu-admin:9095
+    healthcheck:
+      test: [ "CMD-SHELL", "wget -q -O - http://localhost:8011/actuator/health | grep UP || exit 1" ]
+      timeout: 2s
+      retries: 3
+      start_period: 5s
+    ports:
+      - "8011:8011"
+      - "8005:8005"
+    networks:
+      - shenyu
+    depends_on:
+      shenyu-integrated-test-brpc:
+        condition: service_healthy
+
+  shenyu-integrated-test-brpc:
+    container_name: shenyu-integrated-test-brpc
+    image: apache/shenyu-integrated-test-brpc:latest
+    restart: always
+    deploy:
+      resources:
+        limits:
+          memory: 2048M
+    environment:
+      - shenyu.sync.websocket.urls=ws://shenyu-admin:9095/websocket
+    depends_on:
+      shenyu-admin:

Review Comment:
   Here, we should depends on `shenyu-examples-brpc-service`.



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#issuecomment-1434137180

   see https://github.com/apache/shenyu/pull/4319


-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] codecov-commenter commented on pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#issuecomment-1416702157

   # [Codecov](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#4352](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (21b1a89) into [master](https://codecov.io/gh/apache/shenyu/commit/23876bf67c6075b6e865611c5fa9affc82af1a6c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (23876bf) will **decrease** coverage by `0.03%`.
   > The diff coverage is `16.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4352      +/-   ##
   ============================================
   - Coverage     68.25%   68.22%   -0.03%     
   - Complexity     7516     7517       +1     
   ============================================
     Files          1020     1020              
     Lines         28831    28832       +1     
     Branches       2585     2582       -3     
   ============================================
   - Hits          19678    19671       -7     
   - Misses         7613     7622       +9     
   + Partials       1540     1539       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...che/shenyu/plugin/brpc/proxy/BrpcProxyService.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWJycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vYnJwYy9wcm94eS9CcnBjUHJveHlTZXJ2aWNlLmphdmE=) | `2.17% <0.00%> (+0.13%)` | :arrow_up: |
   | [...enyu/plugin/brpc/cache/ApplicationConfigCache.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWJycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vYnJwYy9jYWNoZS9BcHBsaWNhdGlvbkNvbmZpZ0NhY2hlLmphdmE=) | `45.05% <17.39%> (-4.30%)` | :arrow_down: |
   | [...a/org/apache/shenyu/common/utils/VersionUtils.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hlbnl1L2NvbW1vbi91dGlscy9WZXJzaW9uVXRpbHMuamF2YQ==) | `21.27% <0.00%> (-6.39%)` | :arrow_down: |
   | [...henyu/plugin/grpc/resolver/ShenyuNameResolver.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWdycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vZ3JwYy9yZXNvbHZlci9TaGVueXVOYW1lUmVzb2x2ZXIuamF2YQ==) | `58.51% <0.00%> (-5.32%)` | :arrow_down: |
   | [...yu/common/dto/convert/rule/Resilience4JHandle.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hlbnl1L2NvbW1vbi9kdG8vY29udmVydC9ydWxlL1Jlc2lsaWVuY2U0SkhhbmRsZS5qYXZh) | `71.60% <0.00%> (-3.71%)` | :arrow_down: |
   | [...a/org/apache/shenyu/plugin/mock/util/MockUtil.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLW1vY2svc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vbW9jay91dGlsL01vY2tVdGlsLmphdmE=) | `77.55% <0.00%> (-2.05%)` | :arrow_down: |
   | [...rg/apache/shenyu/plugin/hystrix/HystrixPlugin.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWh5c3RyaXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vaHlzdHJpeC9IeXN0cml4UGx1Z2luLmphdmE=) | `75.86% <0.00%> (-0.81%)` | :arrow_down: |
   | [...yu/admin/service/impl/PluginHandleServiceImpl.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LWFkbWluL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGVueXUvYWRtaW4vc2VydmljZS9pbXBsL1BsdWdpbkhhbmRsZVNlcnZpY2VJbXBsLmphdmE=) | `75.47% <0.00%> (-0.46%)` | :arrow_down: |
   | [.../plugin/grpc/loadbalance/AbstractLoadBalancer.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWdycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vZ3JwYy9sb2FkYmFsYW5jZS9BYnN0cmFjdExvYWRCYWxhbmNlci5qYXZh) | `70.32% <0.00%> (-0.33%)` | :arrow_down: |
   | [...enyu/plugin/sofa/cache/ApplicationConfigCache.java](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLXNvZmEvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vc29mYS9jYWNoZS9BcHBsaWNhdGlvbkNvbmZpZ0NhY2hlLmphdmE=) | `69.44% <0.00%> (-0.29%)` | :arrow_down: |
   | ... and [10 more](https://codecov.io/gh/apache/shenyu/pull/4352?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103581183


##########
shenyu-integrated-test/shenyu-integrated-test-brpc/src/main/resources/application-local.yml:
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+server:
+  port: 9195
+  address: 0.0.0.0
+
+spring:
+  main:
+    allow-bean-definition-overriding: true
+  application:
+    name: shenyu-bootstrap
+
+management:
+  health:
+    defaults:
+      enabled: false
+
+shenyu:
+  switchConfig:
+    local: true
+  cross:
+    enabled: true
+  sync:
+    websocket:
+      urls: ws://shenyu-admin:9095/websocket

Review Comment:
   `ws://shenyu-admin:9095/websocket` can be change to `ws://localhost:9095/websocket`, because we had set the  config in `docker-compose.yml`



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103585318


##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/cache/ApplicationConfigCache.java:
##########
@@ -116,9 +139,7 @@ public AsyncGenericService build(final MetaData metaData) {
         if (Objects.isNull(clientConfig)) {
             throw new UnsupportedOperationException("unsupport!!");
         }

Review Comment:
   When `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#init` is not called, try to initialize `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#initService` , `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#build ` will throw `UnsupportedOperationException`.
   
   Therefore, we should delay the action of building `AsyncGenericService` until the `runtime`. Of course, caching `AsyncGenericService`  is recommended.



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103586434


##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/proxy/BrpcProxyService.java:
##########
@@ -81,8 +81,7 @@ public Mono<Object> genericInvoker(final String body, final MetaData metaData, f
             }
         }
         initThreadPool();
-        CompletableFuture<Object> future = null;
-        future = new CompletableFuture<>().supplyAsync(() -> getValue(metaData, params), threadPool);
+        CompletableFuture<Object> future = CompletableFuture.supplyAsync(() -> getValue(metaData, params), threadPool);

Review Comment:
   In the future, we should use `com.baidu.cloud.starlight.api.rpc.threadpool.ThreadPoolFactory` SPI to impl it, this is a todo list.



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103586434


##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/proxy/BrpcProxyService.java:
##########
@@ -81,8 +81,7 @@ public Mono<Object> genericInvoker(final String body, final MetaData metaData, f
             }
         }
         initThreadPool();
-        CompletableFuture<Object> future = null;
-        future = new CompletableFuture<>().supplyAsync(() -> getValue(metaData, params), threadPool);
+        CompletableFuture<Object> future = CompletableFuture.supplyAsync(() -> getValue(metaData, params), threadPool);

Review Comment:
   In the future, we use `com.baidu.cloud.starlight.api.rpc.threadpool.ThreadPoolFactory` SPI to impl it, this is a todo list.



-- 
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: notifications-unsubscribe@shenyu.apache.org

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

[GitHub] [shenyu] dragon-zhang commented on a diff in pull request #4352: [ISSUE #3450] add brpc integrated test

Posted by "dragon-zhang (via GitHub)" <gi...@apache.org>.
dragon-zhang commented on code in PR #4352:
URL: https://github.com/apache/shenyu/pull/4352#discussion_r1103585772


##########
shenyu-plugin/shenyu-plugin-brpc/src/main/java/org/apache/shenyu/plugin/brpc/cache/ApplicationConfigCache.java:
##########
@@ -152,26 +173,44 @@ public AsyncGenericService build(final MetaData metaData) {
      */
     public AsyncGenericService get(final String path) {
         try {
-            return cache.get(path);
+            return serviceCache.get(path);
         } catch (ExecutionException e) {
             throw new ShenyuBrpcPluginException(e.getCause());
         }
     }
 
+    /**
+     * init brpc service config.
+     *
+     * @param metaData the meta data
+     * @return service config
+     */
+    public ServiceConfig initServiceConfig(final MetaData metaData) {

Review Comment:
   In `org.apache.shenyu.plugin.brpc.handler.BrpcMetaDataHandler#handle`, maybe just change `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#initService` to `org.apache.shenyu.plugin.brpc.cache.ApplicationConfigCache#initServiceConfig` ?



-- 
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: notifications-unsubscribe@shenyu.apache.org

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