[GitHub] [apisix] moonming commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API

[GitBox <gi...@apache.org>]

moonming commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r471343434



##########
File path: .travis/linux_openresty_v2_runner.sh
##########
@@ -0,0 +1,198 @@
+#!/usr/bin/env bash

Review comment:
       What is the difference between this file and `linux_openresty_runner.sh`? 

##########
File path: conf/config.yaml
##########
@@ -129,6 +129,8 @@ etcd:
     - "http://127.0.0.1:2379"     # multiple etcd address
   prefix: "/apisix"               # apisix configurations prefix
   timeout: 30                     # 30 seconds
+  version: "v3"                   # etcd version: v2/v3
+  api_prefix: "/v3alpha"          # 3.2 -> "/v3alpha", 3.3 -> "/v3beta", 3.4+ -> "/v3"

Review comment:
       So we don't need to add configures for that.

##########
File path: .github/workflows/build.yml
##########
@@ -12,7 +12,7 @@ jobs:
       fail-fast: false
       matrix:
         platform: [ubuntu-18.04]
-        os_name: [linux_openresty, linux_tengine, linux_apisix_master_luarocks, linux_apisix_current_luarocks, linux_openresty_mtls]
+        os_name: [linux_openresty, linux_tengine, linux_apisix_master_luarocks, linux_apisix_current_luarocks, linux_openresty_mtls, linux_openresty_v2]

Review comment:
       what is `linux_openresty_v2`? I think we need a meaningful name

##########
File path: conf/config.yaml
##########
@@ -129,6 +129,8 @@ etcd:
     - "http://127.0.0.1:2379"     # multiple etcd address
   prefix: "/apisix"               # apisix configurations prefix
   timeout: 30                     # 30 seconds
+  version: "v3"                   # etcd version: v2/v3
+  api_prefix: "/v3alpha"          # 3.2 -> "/v3alpha", 3.3 -> "/v3beta", 3.4+ -> "/v3"

Review comment:
       we should only support `v3` and `/v3`

##########
File path: apisix/core.lua
##########
@@ -19,7 +19,9 @@ local local_conf = require("apisix.core.config_local").local_conf()
 
 local config_center = local_conf.apisix and local_conf.apisix.config_center
                       or "etcd"
+local etcd_version = local_conf.etcd.version == "v3" and "_v3" or ""

Review comment:
       IMO, we should ONLY supports etcd v3, not etcd v2.




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