You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/10/29 05:13:29 UTC

[GitHub] [apisix] shoogoome opened a new pull request #2556: feature:support multi service discovery

shoogoome opened a new pull request #2556:
URL: https://github.com/apache/apisix/pull/2556


   ### What this PR does / why we need it:
   support multi service discovery
   
   ### Pre-submission checklist:
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible?
   


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

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



[GitHub] [apisix] membphis commented on pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#issuecomment-718377017


   @qiujiayu do you have time to look at this PR?


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

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



[GitHub] [apisix] nic-chen commented on a change in pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#discussion_r514656355



##########
File path: apisix/discovery/eureka.lua
##########
@@ -66,7 +66,8 @@ local _M = {
 
 
 local function service_info()
-    local host = local_conf.eureka and local_conf.eureka.host
+    local host = local_conf.discovery and
+        local_conf.discovery.eureka and local_conf.discovery.eureka.host

Review comment:
       Could we support different instances for a same type discovery?




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

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



[GitHub] [apisix] shoogoome commented on pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
shoogoome commented on pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#issuecomment-718787565


   > @shoogoome please take a look at the output of CI: https://github.com/apache/apisix/pull/2556/checks?check_run_id=1324835665#step:10:187
   
   thx


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

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



[GitHub] [apisix] shoogoome commented on a change in pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
shoogoome commented on a change in pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#discussion_r514298980



##########
File path: t/discovery/eureka.t
##########
@@ -26,7 +26,8 @@ apisix:
   node_listen: 1984
   config_center: yaml
   enable_admin: false
-  discovery: eureka
+  discovery:
+    - eureka
 

Review comment:
       done




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

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



[GitHub] [apisix] ShiningRush commented on a change in pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#discussion_r514690061



##########
File path: apisix/discovery/eureka.lua
##########
@@ -66,7 +66,8 @@ local _M = {
 
 
 local function service_info()
-    local host = local_conf.eureka and local_conf.eureka.host
+    local host = local_conf.discovery and
+        local_conf.discovery.eureka and local_conf.discovery.eureka.host

Review comment:
       This scenario is unreasonable and will not be supported for now. 
   However, users can still handle this scenario by creating new instances of different gateways, I think it is enough.




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

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



[GitHub] [apisix] ShiningRush commented on a change in pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#discussion_r514066039



##########
File path: t/discovery/eureka.t
##########
@@ -26,7 +26,8 @@ apisix:
   node_listen: 1984
   config_center: yaml
   enable_admin: false
-  discovery: eureka
+  discovery:
+    - eureka
 

Review comment:
       We can also move all discovery configuration to here, it would be better.Such as 
   ```
   discovery:
     eureka:
       host: [""]
       prefix: ""
       ...
     consul:
       uri: ""
   ```




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

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



[GitHub] [apisix] membphis commented on pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#issuecomment-718377287


   @shoogoome please take a look at the output of CI: https://github.com/apache/apisix/pull/2556/checks?check_run_id=1324835665#step:10:187


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

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



[GitHub] [apisix] ShiningRush commented on pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
ShiningRush commented on pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#issuecomment-718443824


   You should init dicovery at https://github.com/apache/apisix/blob/master/apisix/init.lua#L94.


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

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



[GitHub] [apisix] ShiningRush commented on a change in pull request #2556: feature:support multi service discovery

Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #2556:
URL: https://github.com/apache/apisix/pull/2556#discussion_r514066039



##########
File path: t/discovery/eureka.t
##########
@@ -26,7 +26,8 @@ apisix:
   node_listen: 1984
   config_center: yaml
   enable_admin: false
-  discovery: eureka
+  discovery:
+    - eureka
 

Review comment:
       We can also move all eureka configuration to here, it would be better.




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

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