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 if older deployments defined a 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 a 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