Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| Lifecycle: LifecycleFeatureConfig{ | ||
| Enabled: false, | ||
| }, | ||
| RateLimiting: RateLimitingFeatureConfig{ |
There was a problem hiding this comment.
Rate limiting requires Redis (via localCache), but the Redis service in docker-compose.yaml does not include a feature-rate-limiting profile. If a user enables only rate limiting (without utapi, CRR, etc.), Redis won't start and cloudserver will fail to connect. A feature-rate-limiting profile needs to be added to the Redis service, and getComposeProfiles in cmd/util.go needs a corresponding entry for RateLimiting.Enabled.
— Claude Code
Review by Claude Code |
| "limit": {{ .Features.RateLimiting.Account.RequestsPerSecond.Limit }}, | ||
| "burstCapacity": {{ .Features.RateLimiting.Account.RequestsPerSecond.BurstCapacity }} | ||
| } | ||
| }, |
There was a problem hiding this comment.
configCacheTTL is hardcoded to 30000 here instead of using the template variable. The bucket section correctly uses {{ .Features.RateLimiting.Bucket.ConfigCacheTTL }}, but the account section ignores the config value.
| }, | |
| "configCacheTTL": {{ .Features.RateLimiting.Account.ConfigCacheTTL }} |
— Claude Code
| {{- end }} | ||
| "configCacheTTL": 30000 | ||
| }, | ||
| "error": { |
There was a problem hiding this comment.
Same hardcoded configCacheTTL issue as in config-v9.json — should use {{ .Features.RateLimiting.Account.ConfigCacheTTL }} to respect user config.
| "error": { | |
| "configCacheTTL": {{ .Features.RateLimiting.Account.ConfigCacheTTL }} |
— Claude Code
| {{ if .Features.Utapi.Enabled }} | ||
| {{ if or .Features.Utapi.Enabled .Features.RateLimiting.Enabled }} | ||
| "localCache": { | ||
| "host": "localhost", |
There was a problem hiding this comment.
The templates now require Redis (localCache) when rate limiting is enabled, but the Redis service in docker-compose.yaml does not include a rate-limiting profile, and getComposeProfiles() in cmd/util.go has no entry for features.rate_limiting. If a user enables only rate limiting (no utapi, crr, notifications, migration, or lifecycle), Redis will not start and cloudserver will fail to connect.
You need to: (1) add a profile like feature-rate-limiting to the Redis service in docker-compose.yaml, and (2) add a corresponding block in getComposeProfiles() to activate it when cfg.Features.RateLimiting.Enabled is true.
— Claude Code
Review by Claude Code |
tmacro
force-pushed
the
improvement/WKBCH-23/rate_limit_support
branch
from
551d6a2 to
a3d5a32
Compare
| @@ -275,6 +299,19 @@ func DefaultEnvironmentConfig() EnvironmentConfig { | |||
| Lifecycle: LifecycleFeatureConfig{ | |||
| Enabled: false, | |||
| }, | |||
There was a problem hiding this comment.
Every other feature in DefaultEnvironmentConfig() explicitly sets Enabled: false (e.g., Utapi, Migration, Lifecycle). While Go's zero value handles this correctly, adding it here keeps the defaults self-documenting and consistent.
| }, | |
| RateLimiting: RateLimitingFeatureConfig{ | |
| Enabled: false, | |
| Bucket: RateLimitingDefaultLimits{ |
— Claude Code
|
tmacro
force-pushed
the
improvement/WKBCH-23/rate_limit_support
branch
from
a3d5a32 to
7e1c043
Compare
|
|
||
| func templateFile(templates fs.FS, path string, data any) ([]byte, error) { | ||
| tmpl, err := template.ParseFS(templates, path) | ||
| tmpl, err := template.New("base").Funcs(sprig.FuncMap()).ParseFS(templates, path) |
There was a problem hiding this comment.
template.New("base") creates a template named "base", but ParseFS names parsed templates after their file basename (e.g., "config-v7.json"). When Execute is called on the "base" template, it will fail with "base" is an incomplete or empty template because the actual content lives in an associated template with a different name.
The original package-level template.ParseFS avoided this because it named the root template after the first file.
| tmpl, err := template.New("base").Funcs(sprig.FuncMap()).ParseFS(templates, path) | |
| tmpl, err := template.New(filepath.Base(path)).Funcs(sprig.FuncMap()).ParseFS(templates, path) |
— Claude Code
| volumes: | ||
| - ./config/vault/:/secrets | ||
| profiles: | ||
| - base |
There was a problem hiding this comment.
setup-cloudserver is in the base profile, so it runs for ALL environments — even those not using rate limiting. This forces every user to pull SCUBA_IMAGE and run the rate-limit service user creation script. It should use feature-rate-limiting profile, matching the pattern of setup-scuba in feature-scuba.
— Claude Code
| network_mode: host | ||
| depends_on: | ||
| setup-cloudserver: | ||
| condition: service_completed_successfully |
There was a problem hiding this comment.
cloudserver unconditionally depends on setup-cloudserver, but setup-cloudserver should be in the feature-rate-limiting profile (not base). When rate limiting is disabled and setup-cloudserver is not started, Docker Compose will error on this unresolved dependency. This depends_on should be removed; instead, have cloudserver depend on setup-cloudserver only when both are in the same active profile set, or restructure the dependency chain so setup-cloudserver runs independently in the rate-limiting profile.
— Claude Code
Review by Claude Code |
3 participants
No description provided.