abhilashmandaliya edited a comment on pull request #11697:
URL: https://github.com/apache/pulsar/pull/11697#issuecomment-901099857


   > > It seems that it's possible to just override the setter 
"setBacklogQuotaDefaultLimitGB" in ServiceConfiguration class and update the 
backlogQuotaDefaultLimitBytes field on the fly. It's necessary to keep the 
backlogQuotaDefaultLimitGB field so that the FieldContext annotation can be 
used for documenting the field.
   > > I would add a getBacklogQuotaDefaultLimitGB() method with private access 
to ensure that the method isn't used anywhere in the code directly anymore. 
ServiceConfiguration isn't part of any public API, there's no need to keep API 
backwards compatibility for the class itself. As long as the configuration file 
format is supported, it's sufficient.
   > 
   > I take this back. It's not currently possible to use setters with the 
configuration loading logic.
   > 
   > Since BacklogQuotaManager is the only location of the logic in production 
code. Please add a test for calculation logic. It should be pretty 
straightforward to add the unit test that verifies that 
getDefaultQuota().getLimitSize() returns the correct bytes when the 
configuration is using a fractional GB value.
   
   Thanks. I have added a unit test.


-- 
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: commits-unsubscr...@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to