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.
| @@ -68,7 +68,7 @@ | |||
| public class OAuth2Credentials extends Credentials { | |||
|
|
|||
| 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.
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. |
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.