dimas-b commented on code in PR #3507:
URL: https://github.com/apache/polaris/pull/3507#discussion_r2717409702


##########
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:
   This could be a breaking change is older deployments defined the config 
value for it in `application.properties` (which is introspected and validated 
by Quarkus, IIRC). A missing "receiver" for those configs could be a runtime 
error (and config is under strict guarantees in our standing [evolution 
guidelines](https://polaris.apache.org/in-dev/unreleased/evolution/)).
   
   Could we keep and deprecated receiver just to improve backward compatibility?



-- 
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]

Reply via email to