snazy commented on code in PR #3507:
URL: https://github.com/apache/polaris/pull/3507#discussion_r2720960249
##########
runtime/service/src/main/java/org/apache/polaris/service/ratelimiter/TokenBucketConfiguration.java:
##########
@@ -19,15 +19,15 @@
package org.apache.polaris.service.ratelimiter;
import io.smallrye.config.ConfigMapping;
-import java.time.Duration;
@ConfigMapping(prefix = "polaris.rate-limiter.token-bucket")
public interface TokenBucketConfiguration {
+ /**
+ * Number of allowed requests per second per realm. The value <em>must</em>
be greater than zero.
+ */
long requestsPerSecond();
- Duration window();
Review Comment:
Re-added the `window` option as "deprecated for removal" and added a note to
the CHANGELOG.
##########
helm/polaris/README.md:
##########
@@ -416,7 +416,6 @@ ct install --namespace polaris --charts ./helm/polaris
| rateLimiter.tokenBucket | object |
`{"requestsPerSecond":9999,"type":"default","window":"PT10S"}` | The
configuration for the default rate limiter, which uses the token bucket
algorithm with one bucket per realm. |
| rateLimiter.tokenBucket.requestsPerSecond | int | `9999` | The maximum
number of requests per second allowed for each realm. |
| rateLimiter.tokenBucket.type | string | `"default"` | The type of the token
bucket rate limiter. Only the default type is supported out of the box. |
-| rateLimiter.tokenBucket.window | string | `"PT10S"` | The time window. |
Review Comment:
Added notes to the CHANGELOG
##########
helm/polaris/templates/configmap.yaml:
##########
@@ -64,7 +64,6 @@ data:
{{- if ne .Values.rateLimiter.type "no-op" -}}
{{- $_ = set $map "polaris.rate-limiter.token-bucket.type"
.Values.rateLimiter.tokenBucket.type -}}
{{- $_ = set $map "polaris.rate-limiter.token-bucket.requests-per-second"
.Values.rateLimiter.tokenBucket.requestsPerSecond -}}
- {{- $_ = set $map "polaris.rate-limiter.token-bucket.window"
.Values.rateLimiter.tokenBucket.window -}}
Review Comment:
Those are already removed (see below)
##########
helm/polaris/templates/configmap.yaml:
##########
@@ -64,7 +64,6 @@ data:
{{- if ne .Values.rateLimiter.type "no-op" -}}
{{- $_ = set $map "polaris.rate-limiter.token-bucket.type"
.Values.rateLimiter.tokenBucket.type -}}
{{- $_ = set $map "polaris.rate-limiter.token-bucket.requests-per-second"
.Values.rateLimiter.tokenBucket.requestsPerSecond -}}
- {{- $_ = set $map "polaris.rate-limiter.token-bucket.window"
.Values.rateLimiter.tokenBucket.window -}}
Review Comment:
I missed to remove the `window` setting right above in configmap_test tho
--
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]
