Add Redis client utility and corresponding tests - #3172
Closed
Thenujan-Nagaratnam wants to merge 25 commits into
Closed
Add Redis client utility and corresponding tests#3172Thenujan-Nagaratnam wants to merge 25 commits into
Thenujan-Nagaratnam wants to merge 25 commits into
Conversation
Thenujan-Nagaratnam
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Induwara04,
Krishanx92,
PasanT9,
Piumal1999,
RakhithaRR,
Tharsanan1,
Thushani-Jayasekera,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub,
tharindu1st and
thivindu
as code owners
August 7, 2026 04:59
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (50)
📝 WalkthroughWalkthroughAdds upstream retry credential refresh through new SDK policy contracts, Envoy retry translation, and a dedicated policy-engine ChangesUpstream retry refresh and SDK contracts
Policy-driven upstream authentication
Shared Redis client
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/core/utils/redisclient/redisclient_test.go`:
- Around line 123-135: Update TestHashPassword to replace the
identical-expression comparison with an assertion that
hashRedisPassword("secret") equals the known SHA-256 hexadecimal digest for
“secret”; retain the other password behavior checks unchanged.
In `@sdk/core/utils/redisclient/redisclient.go`:
- Around line 79-91: Update the client-creation flow around the redisClients
mutex: register the newly created client while the mutex is held, then release
the lock before creating the timeout context and calling c.Ping(ctx). Preserve
the existing-client fast path and ensure deferred unlock behavior does not keep
the registry locked during network I/O.
- Around line 68-77: Update the redisConnKey construction in
GetOrCreateRedisClient to distinguish clients with different TLSConfig,
Protocol, and credential-provider settings. Add safe, comparable fingerprints
for these options, or bypass registry reuse when an option cannot be safely
fingerprinted; preserve existing password handling and ensure differing
connection behavior cannot share a cached client.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 557670c7-3f35-4077-be82-c233c67f2606
⛔ Files ignored due to path filters (1)
sdk/core/go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
sdk/core/go.modsdk/core/utils/redisclient/redisclient.gosdk/core/utils/redisclient/redisclient_test.go
…tion handling with TLS and credentials provider
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/core/utils/redisclient/redisclient_test.go`:
- Line 187: Update the deferred listener cleanup around ln.Close to explicitly
handle or discard its returned error, and update the slow-call results at the
referenced synchronization-test locations with blank assignments when
intentionally unused. Ensure all affected calls satisfy errcheck without
changing the test behavior.
- Around line 188-213: The test’s time.Sleep does not guarantee the slow Redis
client goroutine has reached Ping before the fast call is measured. Add a
synchronization channel signaled immediately after ln.Accept() succeeds, then
wait for that signal with a bounded test timeout before creating the unrelated
client, preserving the existing slow-client setup and cleanup behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a79bb6e9-39aa-494e-8dd9-268f1d8ae02e
📒 Files selected for processing (2)
sdk/core/utils/redisclient/redisclient.gosdk/core/utils/redisclient/redisclient_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/core/utils/redisclient/redisclient.go
…re accurate timeout signaling
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
sdk/core/utils/redisclient/redisclient.go (1)
88-98: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftFingerprint every behavior-changing
redis.Optionsfield.Lines 88-98 omit options such as
PoolTimeout, idle/active connection limits, retry settings,PoolFIFO,Dialer, andOnConnect. Two callers can therefore receive the same client while requiring different pool, retry, or dial behavior. Bypass reuse for non-fingerprintable hooks. Include every relevant scalar option inredisConnKey. Add a regression test with an omitted scalar option.#!/bin/bash set -euo pipefail go doc github.com/redis/go-redis/v9.Options rg -n -C 5 'redisConnKey|GetOrCreateRedisClient|redis\.Options' sdk/core/utils/redisclient🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/core/utils/redisclient/redisclient.go` around lines 88 - 98, Update redisConnKey and the client reuse logic around GetOrCreateRedisClient to fingerprint every behavior-changing redis.Options field, including pool limits/timeouts, retry settings, PoolFIFO, and other relevant scalar options. Treat non-fingerprintable hooks such as Dialer and OnConnect as ineligible for reuse rather than comparing or hashing them. Add a regression test proving that differing omitted scalar options produce distinct clients.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/gateway-runtime/policy-engine/go.mod`:
- Around line 54-59: Update the OpenTelemetry indirect dependencies
go.opentelemetry.io/otel/exporters/otlp/otlptrace and
go.opentelemetry.io/otel/metric to v1.45.0, or document why either cannot be
updated; then run the full policy checks govulncheck ./... and go-licenses check
./... for the policy-engine module.
In `@sdk/core/utils/redisclient/redisclient.go`:
- Around line 319-320: Update intParam’s float64 handling to reject NaN,
infinities, fractional values, and values outside the int range before
conversion; return the existing parameter-validation error for invalid inputs
and only convert integral, in-range values.
- Around line 281-290: Update the Redis options construction to build Addr with
net.JoinHostPort(host, strconv.Itoa(port)) instead of fmt.Sprintf, ensuring IPv6
hosts are bracketed correctly; add a configuration test covering an IPv6 host
and verifying the resulting address.
- Around line 119-122: Replace the fixed defaultSharedPingTimeout used by
InitFromConfig before newAndPingClient with a timeout derived from the
configured Redis dial and command timeouts, adding a small safety margin. Ensure
the resulting ping context remains active at least as long as the connection
attempt permitted by redis.Options.DialTimeout.
---
Duplicate comments:
In `@sdk/core/utils/redisclient/redisclient.go`:
- Around line 88-98: Update redisConnKey and the client reuse logic around
GetOrCreateRedisClient to fingerprint every behavior-changing redis.Options
field, including pool limits/timeouts, retry settings, PoolFIFO, and other
relevant scalar options. Treat non-fingerprintable hooks such as Dialer and
OnConnect as ineligible for reuse rather than comparing or hashing them. Add a
regression test proving that differing omitted scalar options produce distinct
clients.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0325bb96-1e15-40bf-be6e-5ff40beab007
⛔ Files ignored due to path filters (1)
gateway/gateway-runtime/policy-engine/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
gateway/gateway-runtime/policy-engine/cmd/policy-engine/main.gogateway/gateway-runtime/policy-engine/go.modsdk/core/utils/redisclient/redisclient.gosdk/core/utils/redisclient/redisclient_test.gosdk/core/utils/redisclient/testing.go
Comment on lines
+54
to
+59
| github.com/redis/go-redis/v9 v9.22.0 // indirect | ||
| github.com/stoewer/go-strcase v1.3.1 // indirect | ||
| go.opentelemetry.io/auto/sdk v1.2.1 // indirect | ||
| go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.44.0 // indirect | ||
| go.opentelemetry.io/otel/metric v1.44.0 // indirect | ||
| go.uber.org/atomic v1.11.0 // indirect |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
go mod graph
go list -m all
govulncheck ./...
go-licenses check ./...
go list -m -json all |
jq -r 'select(.Version != null) | "\(.Path)@\(.Version)"'Repository: wso2/api-platform
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
file="gateway/gateway-runtime/policy-engine/go.mod"
printf '%s\n' '--- dependency block ---'
sed -n '1,90p' "$file"
printf '%s\n' '--- focused diff ---'
git diff -- "$file" | sed -n '1,180p'
printf '%s\n' '--- tool availability ---'
for tool in go govulncheck go-licenses jq curl; do
if command -v "$tool" >/dev/null 2>&1; then
printf '%s: %s\n' "$tool" "$(command -v "$tool")"
else
printf '%s: unavailable\n' "$tool"
fi
done
printf '%s\n' '--- module metadata ---'
(
cd "$(dirname "$file")"
go version 2>&1 || true
go list -m -json github.com/redis/go-redis/v9 github.com/stoewer/go-strcase go.opentelemetry.io/auto/sdk go.opentelemetry.io/otel/exporters/otlp/otlptrace go.opentelemetry.io/otel/metric go.uber.org/atomic 2>&1 || true
)
printf '%s\n' '--- focused graph edges ---'
(
cd "$(dirname "$file")"
go mod graph 2>/dev/null | grep -E '(^| )((github.com/redis/go-redis/v9|github.com/stoewer/go-strcase|go.opentelemetry.io/auto/sdk|go.opentelemetry.io/otel/exporters/otlp/otlptrace|go.opentelemetry.io/otel/metric|go.uber.org/atomic))@' | head -n 120 || true
)Repository: wso2/api-platform
Length of output: 17047
🏁 Script executed:
#!/bin/bash
set -u
modules=(
"github.com/redis/go-redis/v9@v9.22.0"
"github.com/stoewer/go-strcase@v1.3.1"
"go.opentelemetry.io/auto/sdk@v1.2.1"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace@v1.44.0"
"go.opentelemetry.io/otel/metric@v1.44.0"
"go.uber.org/atomic@v1.11.0"
)
printf '%s\n' '--- available updates ---'
(
cd gateway/gateway-runtime/policy-engine
go list -m -u "${modules[@]}" 2>&1 || true
)
printf '%s\n' '--- OSV results ---'
python3 - <<'PY'
import json
import urllib.request
queries = [
("github.com/redis/go-redis/v9", "v9.22.0"),
("github.com/stoewer/go-strcase", "v1.3.1"),
("go.opentelemetry.io/auto/sdk", "v1.2.1"),
("go.opentelemetry.io/otel/exporters/otlp/otlptrace", "v1.44.0"),
("go.opentelemetry.io/otel/metric", "v1.44.0"),
("go.uber.org/atomic", "v1.11.0"),
]
payload = {"queries": [{"package": {"name": p, "ecosystem": "Go"}, "version": v}
for p, v in queries]}
req = urllib.request.Request(
"https://api.osv.dev/v1/querybatch",
data=json.dumps(payload).encode(),
headers={"Content-Type": "application/json"},
)
try:
with urllib.request.urlopen(req, timeout=30) as response:
data = json.load(response)
for (package, version), result in zip(queries, data.get("results", [])):
vulns = result.get("vulns", [])
print(f"{package}@{version}: " +
("no OSV vulnerabilities" if not vulns else
" ".join(v.get("id", "unknown") for v in vulns)))
except Exception as exc:
print(f"OSV query failed: {exc}")
PY
printf '%s\n' '--- module proxy release lists ---'
for module in \
github.com/redis/go-redis/v9 \
github.com/stoewer/go-strcase \
go.opentelemetry.io/auto/sdk \
go.opentelemetry.io/otel/exporters/otlp/otlptrace \
go.opentelemetry.io/otel/metric \
go.uber.org/atomic
do
encoded=$(printf '%s' "$module" | sed 's|/|%2F|g')
printf '%s: ' "$module"
curl -fsSL --max-time 20 "https://proxy.golang.org/$encoded/@v/list" 2>/dev/null |
tail -n 5 | tr '\n' ' '
printf '\n'
doneRepository: wso2/api-platform
Length of output: 1098
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- OSV results (TLS fallback only for this read-only query) ---'
python3 - <<'PY'
import json
import subprocess
queries = [
("github.com/redis/go-redis/v9", "v9.22.0"),
("github.com/stoewer/go-strcase", "v1.3.1"),
("go.opentelemetry.io/auto/sdk", "v1.2.1"),
("go.opentelemetry.io/otel/exporters/otlp/otlptrace", "v1.44.0"),
("go.opentelemetry.io/otel/metric", "v1.44.0"),
("go.uber.org/atomic", "v1.11.0"),
]
payload = json.dumps({
"queries": [{"package": {"name": p, "ecosystem": "Go"}, "version": v}
for p, v in queries]
})
proc = subprocess.run(
["curl", "-kfsSL", "--max-time", "30",
"-H", "Content-Type: application/json",
"--data-binary", payload,
"https://api.osv.dev/v1/querybatch"],
text=True, capture_output=True
)
if proc.returncode:
print(f"OSV query failed: {proc.stderr.strip()}")
else:
try:
data = json.loads(proc.stdout)
for (package, version), result in zip(queries, data.get("results", [])):
vulns = result.get("vulns", [])
print(f"{package}@{version}: " +
("no OSV vulnerabilities" if not vulns else
" ".join(v.get("id", "unknown") for v in vulns)))
except Exception as exc:
print(f"OSV response parse failed: {exc}")
PY
printf '%s\n' '--- module licenses ---'
packages=(
"github.com/redis/go-redis/v9@v9.22.0"
"github.com/stoewer/go-strcase@v1.3.1"
"go.opentelemetry.io/auto/sdk@v1.2.1"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace@v1.44.0"
"go.opentelemetry.io/otel/metric@v1.44.0"
"go.uber.org/atomic@v1.11.0"
)
for module in "${packages[@]}"; do
dir=$(
cd gateway/gateway-runtime/policy-engine
go list -m -json "$module" 2>/dev/null |
sed -n 's/^ "Dir": "\(.*\)",$/\1/p'
)
printf '%s\n' "$module"
if [ -n "$dir" ] && [ -d "$dir" ]; then
find "$dir" -maxdepth 1 -type f \
\( -iname 'license' -o -iname 'license.*' -o -iname 'copying' -o -iname 'notice*' \) \
-printf ' %f\n' | sort
else
printf ' module source unavailable\n'
fi
doneRepository: wso2/api-platform
Length of output: 922
🏁 Script executed:
#!/bin/bash
set -u
packages=(
"github.com/redis/go-redis/v9@v9.22.0"
"github.com/stoewer/go-strcase@v1.3.1"
"go.opentelemetry.io/auto/sdk@v1.2.1"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace@v1.44.0"
"go.opentelemetry.io/otel/metric@v1.44.0"
"go.uber.org/atomic@v1.11.0"
)
for module in "${packages[@]}"; do
dir=$(
cd gateway/gateway-runtime/policy-engine
go list -m -json "$module" 2>/dev/null |
sed -n 's/^ "Dir": "\(.*\)",$/\1/p'
)
printf '\n--- %s ---\n' "$module"
if [ -n "$dir" ] && [ -d "$dir" ]; then
license=$(find "$dir" -maxdepth 1 -type f \
\( -iname 'license' -o -iname 'license.*' -o -iname 'copying' \) |
sort | head -n 1)
if [ -n "$license" ]; then
sed -n '1,12p' "$license"
else
printf '%s\n' 'license file not found'
fi
else
printf '%s\n' 'module source unavailable'
fi
doneRepository: wso2/api-platform
Length of output: 3602
Update the OpenTelemetry indirect dependencies and run the full policy checks.
go.opentelemetry.io/otel/exporters/otlp/otlptrace and go.opentelemetry.io/otel/metric have v1.45.0 updates available. Use the latest versions or document the exception. Run govulncheck ./... and go-licenses check ./... for the complete graph.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/gateway-runtime/policy-engine/go.mod` around lines 54 - 59, Update
the OpenTelemetry indirect dependencies
go.opentelemetry.io/otel/exporters/otlp/otlptrace and
go.opentelemetry.io/otel/metric to v1.45.0, or document why either cannot be
updated; then run the full policy checks govulncheck ./... and go-licenses check
./... for the policy-engine module.
Source: Coding guidelines
…al-refresh mechanism
…attempt header mutation
…ree function Extract the route-key extraction logic from the (*ExternalProcessorServer).extractRouteKey method body into a new free function extractRouteKeyFromAttributes. This allows the upcoming UpstreamExternalProcessorServer (Task 3) to reuse the identical logic without duplication, since both servers receive the same ext_proc request-attributes shape. - Add extractRouteKeyFromAttributes(req *extprocv3.ProcessingRequest) string as a package-level unexported function in kernel package - Refactor (*ExternalProcessorServer).extractRouteKey to delegate to the new function - Add TestExtractRouteKeyFromAttributes_MissingAttributesReturnsDefault test case - All existing tests pass with no regressions Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… to UpstreamAttemptPolicy
…hutdown on a second socket Adds a second gRPC ext_proc server (kernel.NewUpstreamExternalProcessorServer, from Task 3) alongside the existing downstream ext_proc server, sharing the same uds/tcp serverMode but its own socket path / configurable port (upstream_extproc_port, default 9004) and its own explicit MaxRecvMsgSize/MaxSendMsgSize/MaxConcurrentStreams sized for its headers-only message shape. Wires matching graceful shutdown and UDS socket cleanup, and adds the new port to the shipped config.toml defaults that set extproc_port explicitly.
… LLM configs - Add validateResilienceRetry() to validate retry status codes and numRetries - Update validateResilience() to call validateResilienceRetry in REST validator - Update LLM validator to also call validateResilienceRetry for consistency - Add unit tests for empty statusCodes, valid config, invalid codes, and negative numRetries - Ensure both REST and LLM validators enforce identical retry validation rules
…ilience Add missing test coverage for LLMProxy resilience.retry validation to ensure both Provider and Proxy validation paths enforce identical retry rules: - retry with empty statusCodes is rejected - retry with valid statusCodes and numRetries is accepted These subtests use validProxyWithResilience helper, consistent with existing timeout/idleTimeout test patterns in TestValidateLLMProxy_Resilience.
…silience.retry Threads api.Retry through the resilience timeout-resolution machinery in both xDS translation paths: the legacy createRoute/translateAPIConfig path and the RuntimeDeployConfig path (createRouteFromRDC), which RestApi/LLM Provider/Proxy kinds actually use in production via RestAPITransformer. Without the RDC-path wiring, resilience.retry would validate successfully but silently produce no RetryPolicy for those kinds.
… backing a retry-configured route Adds a per-cluster upstream ext_proc filter (chained with the mandatory terminal envoy.filters.http.upstream_codec filter) to any Envoy cluster backing at least one route with resilience.retry configured, so UpstreamAttemptPolicy-implementing policies get invoked per retry attempt. Eligibility is computed generically from the already-built route.Route objects (native RetryPolicy + static cluster specifier) rather than re-deriving it separately per translation path, which covers both the legacy (translateAPIConfig/createRoute) and RuntimeDeployConfig (translateRuntimeConfig/createRouteFromRDC — the actual production path for RestApi/Mcp/LlmProvider/LlmProxy) paths with one implementation, verified independently for each with dedicated tests.
…e token refresh Adds OnUpstreamAttemptRequestHeaders, invoked once per upstream dial attempt (including Envoy-native retries). Purges the cached token before refetching on attempt 2+, so a retry never resends the token that was just rejected; fails open (no header mutation) on any fetch error so this can only ever help a retry succeed, never add a new failure mode. Also repoints the dev-policies copy's sdk/core replace path at this worktree's sdk/core - the previous path (main api-platform checkout) predates Task 1's UpstreamAttemptPolicy SDK types. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PNpRTGeyGVe66MFoRJkneX
…e-added by mistake) gateway/dev-policies/ is repo-wide gitignored (.gitignore:165) because it's a local filesystem mirror, not a tracked artifact - gateway-controllers is the actual source of truth. The previous commit force-added these files with git add -f, which would let this copy silently diverge from gateway-controllers without git ever flagging it. Untracks the directory (files remain on disk, untouched) without deleting anything. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PNpRTGeyGVe66MFoRJkneX
…hen any route has resilience.retry Without this, Envoy never emits x-envoy-attempt-count on the upstream request, so the upstream ext_proc filter's UpstreamAttemptContext.AttemptCount was silently always 1 - even on real retries - defeating the whole upstream-attempt refresh mechanism (oauth2-generator's OnUpstreamAttemptRequestHeaders gates entirely on AttemptCount > 1). Confirmed live via e2e verification (Task 10): a native retry on a forced 401 got a clean 200 but silently reused the same already-rejected cached token, since the client-visible status alone doesn't reveal which token was actually resent. Gated on the same clustersNeedingUpstreamFilter signal already used to attach the upstream ext_proc filter itself (true iff at least one route anywhere has resilience.retry configured) - VirtualHost is the only level this Envoy field exists at, so it can't be scoped any tighter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PNpRTGeyGVe66MFoRJkneX
…not globally The previous fix gated this on clustersNeedingUpstreamFilter, which is keyed by cluster name with no vhost affinity - a retry-configured route on one vhost would leak x-envoy-attempt-count onto every other vhost in the same TranslateConfigs call, including unrelated tenants with no retry configured at all. Scoped instead to whether THIS vhost's own routes slice (already in scope inside the vhostMap loop) contains a route with a non-nil RetryPolicy, checked the same way collectClustersNeedingUpstreamFilter already does per-route. Sharpened the two existing tests to assert on specific vhost names rather than looping over "any vhost found" (which couldn't have caught this), and added an explicit two-tenant, two-vhost regression test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PNpRTGeyGVe66MFoRJkneX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new shared Redis client utility to the SDK core, ensuring that identical Redis connection configurations reuse a single
*redis.Clientinstance process-wide, which prevents connection pool leaks and excessive Redis connections. It also adds comprehensive tests for this utility and updates dependencies to support the new functionality.New Redis client sharing utility:
redisclientpackage withGetOrCreateRedisClient, which maintains a process-wide registry of shared*redis.Clientinstances keyed by connection configuration. This ensures connection pools are reused for identical settings and prevents resource leaks. (sdk/core/utils/redisclient/redisclient.go)sdk/core/utils/redisclient/redisclient.go)Testing:
sdk/core/utils/redisclient/redisclient_test.go)Dependency management:
go.modto include new dependencies:go-redis/v9for Redis support andminiredis/v2for in-memory Redis testing, along with related indirect dependencies. (sdk/core/go.mod)