Skip to content

Reuse a valid Google access token in GoogleAuthManager - #4376

Open
chenjian2664 wants to merge 1 commit into
apache:mainfrom
chenjian2664:chenjian2664/bytequay/reuse-valid-google-auth-token
Open

chenjian2664 wants to merge 1 commit into
apache:mainfrom
chenjian2664:chenjian2664/bytequay/reuse-valid-google-auth-token

Conversation

@chenjian2664

Copy link
Copy Markdown
Contributor

Rationale for this change

GoogleAuthManager refreshed its credentials before every REST catalog request, so each catalog call also made a round trip to Google's token endpoint. This was raised in review on #2072. It now refreshes only when the credentials are not valid, which is what google-auth does itself before a request.

Are these changes tested?

Are there any user-facing changes?

GoogleAuthManager refreshed its credentials before every REST catalog
request, so each call made an extra round trip to Google's token
endpoint. Refresh only when the credentials are not valid, which is
what google-auth does itself before a request.
@chenjian2664
chenjian2664 marked this pull request as ready for review November 14, 2026 10:49
Copilot AI balanced review requested due to automatic review settings November 14, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The token-reuse change has no active review issues.

No open issues 🎉

What changed in this PR

Updates GoogleAuthManager to reuse valid access tokens, avoiding an unnecessary Google token-endpoint request before each REST catalog call.

Changes:

  • Refresh credentials only when they are invalid.
  • Add tests for token reuse and refresh near expiry.
  • Mark existing credential mocks as invalid to preserve refresh assertions.
File Description
tests/​catalog/​test_rest.py Updates the credential mock to exercise token refresh.
tests/​catalog/​test_rest_auth.py Adds token-reuse and near-expiry refresh tests; updates existing mocks.
pyiceberg/​catalog/​rest/​auth.py Checks credential validity before refreshing the access token.

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants