michaeljmarshall commented on pull request #13951: URL: https://github.com/apache/pulsar/pull/13951#issuecomment-1033079310
@lhotari - I agree that we need to take care of the life cycle of the thread used for these refresh token commands. In my most recent commit, I changed the behavior so that the application can either supply an executor, or the OAuth2 class will create one (assuming that the feature is enabled, which it is not by default). The one downside here is that this means the feature is not available for brokers, because they create the `AuthenticationOAuth2` class via reflection, and the `configure` method only takes strings. I think we should consider updating how the client authentication providers are configured and then it will probably be easier to use this feature when loading via reflection. > I think that the way to specify the refresh interval by using the "early_token_refresh_percent" configuration parameter is not well suited for all use cases. Would you mind clarifying this use case with an example? On one hand, I accept that percent may be slightly less straight forward, but on the other hand, I think it handles certain edge cases really well. For example, each retrieved token can have a unique `expires_in` value. A percentage naturally handles this variability. Further, there is no guarantee that the min and max refresh interval that you described will fall within the `expires_in` value. Let me know what you think. -- 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]
