nic-6443 commented on code in PR #13461:
URL: https://github.com/apache/apisix/pull/13461#discussion_r3332915284
##########
apisix/plugins/ai-transport/http.lua:
##########
@@ -20,13 +20,19 @@
local core = require("apisix.core")
local http = require("resty.http")
+local rapidjson = require("rapidjson")
+local getmetatable = getmetatable
local ngx_now = ngx.now
local pairs = pairs
local ipairs = ipairs
+local pcall = pcall
local type = type
local str_lower = string.lower
+local tostring = tostring
local _M = {}
+local rapidjson_encode_opts = {sort_keys = true}
+local rapidjson_null = rapidjson.null
Review Comment:
rapidjson is a hard dependency declared in apisix-master-0.rockspec, so it's
always installed with APISIX and can't be missing at runtime. The cjson
fallback is only for per-payload encoding edge cases, not an absent module, so
there's no need to load it optionally.
##########
apisix/plugins/ai-transport/http.lua:
##########
@@ -65,6 +71,50 @@ function _M.construct_forward_headers(ext_opts_headers, ctx)
end
+local function to_rapidjson_value(data)
+ if data == core.json.null then
+ return rapidjson_null
+ end
+
+ if type(data) ~= "table" then
+ return data
+ end
+
+ if getmetatable(data) == core.json.array_mt then
+ local arr = {}
+ for i, v in ipairs(data) do
+ arr[i] = to_rapidjson_value(v)
+ end
+ return rapidjson.array(arr)
+ end
+
+ local obj = {}
+ for k, v in pairs(data) do
+ obj[k] = to_rapidjson_value(v)
+ end
+ return obj
+end
+
+
+local function rapidjson_encode(data)
+ return rapidjson.encode(data, rapidjson_encode_opts)
+end
+
+
+local function encode_body(body)
+ local ok, encoded, err = pcall(rapidjson_encode, to_rapidjson_value(body))
+ if ok and encoded then
+ return encoded
+ end
+
+ core.log.error("failed to encode AI request body with rapidjson: ",
+ ok and (err or "unknown") or tostring(encoded),
+ ", fallback to cjson; LLM cache hit rate may decrease")
+
+ return core.json.encode(body)
+end
Review Comment:
Good catch on the argument evaluation. Fixed in c4fe84bc7 by moving the
to_rapidjson_value conversion inside the pcall'd function, so a conversion
error now also falls back to cjson. The 'rapidjson failed to load' part doesn't
apply here since it's a hard rockspec dependency.
##########
ci/install-lua-rapidjson.sh:
##########
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+RAPIDJSON_VERSION="${RAPIDJSON_VERSION:-0.7.2}"
+ROCKSPEC_VERSION="${ROCKSPEC_VERSION:-0.7.2-1}"
+LUAROCKS_BIN="${LUAROCKS:-luarocks}"
+TREE="${1:-}"
+
+if [ -n "${TREE}" ]; then
+ mkdir -p "${TREE}"
+ TREE="$(cd "${TREE}" && pwd)"
+fi
+
+workdir="$(mktemp -d)"
+trap 'rm -rf "${workdir}"' EXIT
+
+cd "${workdir}"
+"${LUAROCKS_BIN}" unpack rapidjson "${RAPIDJSON_VERSION}"
+cd "rapidjson-${ROCKSPEC_VERSION}/lua-rapidjson"
+
+# lua-rapidjson enables -march=native by default, which can leak CI CPU
+# features into the shipped rapidjson.so.
+sed -i '/add_compile_options(-march=native)/d' CMakeLists.txt
+
+if [ -n "${TREE}" ]; then
+ "${LUAROCKS_BIN}" make "rapidjson-${ROCKSPEC_VERSION}.rockspec"
--tree="${TREE}" --local
+else
+ "${LUAROCKS_BIN}" make "rapidjson-${ROCKSPEC_VERSION}.rockspec"
+fi
Review Comment:
Addressed in c4fe84bc7: the script now fails loudly if the
add_compile_options(-march=native) line is missing (so a native build can't
silently slip through), uses a portable sed -i.bak, and drops the ambiguous
--local when an explicit --tree is given. rapidjson is pinned to 0.7.2-1, so
the matched line is stable.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
