feat: Modify the refresh window to match go/async-token-refresh. Serverless tokens are cached until 4 minutes before expiration, so 4 minutes is the ideal refresh window. #1352
Conversation
…erless tokens are cached until 4 minutes before expiration, so 4 minutes is the ideal refresh window.
| private static final long serialVersionUID = 4556936364828217687L; | ||
| static final Duration DEFAULT_EXPIRATION_MARGIN = Duration.ofMinutes(5); | ||
| static final Duration DEFAULT_EXPIRATION_MARGIN = Duration.ofMinutes(3).plusSeconds(45); | ||
| static final Duration DEFAULT_REFRESH_MARGIN = Duration.ofMinutes(6); |
There was a problem hiding this comment.
I think you want to change both? This to 4 mins or slightly below and expiration to below 3min.
Right now it will start refreshing at 6 mins and get insta cached response for next 2 mins.
TimurSadykov left a comment
TimurSadykov left a comment There was a problem hiding this comment.
Left comment on changing both values
Good point. I have revised the change |
|
| @clundin25 Hey sorry for the late comment... but it looks like those margins aren't covered with tests, could you please add those? There are margin related tests in ComputeCredentialTests |
| @TimurSadykov It seems that it is currently just testing the getters. Is there an additional test you had in mind https://github.com/googleapis/google-auth-library-java/blob/main/oauth2_http/javatests/com/google/auth/oauth2/ComputeEngineCredentialsTest.java#L200-L205? |
| @clundin25 it is a bit more than a getter validation. The test you highlighted validates that compute credentials has their specific margins set. As i understand similar tests are missing for other credentials that supposed to use defaults. It could be that custom margins set up for some credentials and its a bug. |
3 participants
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.