shreemaan-abhishek commented on code in PR #11060:
URL: https://github.com/apache/apisix/pull/11060#discussion_r1544678579
##########
apisix/plugins/request-id.lua:
##########
@@ -79,8 +95,9 @@ local function get_request_id(conf)
if conf.algorithm == "uuid" then
return uuid()
end
+
if conf.algorithm == "nanoid" then
- return nanoid.safe_simple()
+ return nanoid.generate(conf.nanoid.length,conf.nanoid.char_set)
Review Comment:
space needed after comma
##########
t/plugin/request-id.t:
##########
@@ -437,71 +437,3 @@ GET /opentracing
X-Request-ID: 123
--- response_headers
X-Request-ID: 123
-
-
-
-=== TEST 15: add plugin with algorithm nanoid (default uuid)
---- config
- location /t {
- content_by_lua_block {
- local t = require("lib.test_admin").test
- local http = require "resty.http"
- local v = {}
- local ids = {}
- local code, body = t('/apisix/admin/routes/1',
- ngx.HTTP_PUT,
- [[{
- "plugins": {
- "request-id": {
- "algorithm": "nanoid"
Review Comment:
your PR suggests that usage method/configuration for nanoid has changed.
Which means existing users will face incompatibility issues when upgrading to
the newer version. We should avoid such a situation.
##########
apisix/plugins/request-id.lua:
##########
@@ -51,6 +51,22 @@ local schema = {
}
},
default = {}
+ },
+ nanoid = {
Review Comment:
why is this block needed when just replacing the dependency would fix the
issue?
--
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-unsubscr...@apisix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
