Skip to content

Add gRPC timeout to Secret Manager access_secret_version call#63813

Draft
evgeniy-b wants to merge 1 commit intoapache:mainfrom
evgeniy-b:fix-secret-manager-grpc-timeout
Draft

Add gRPC timeout to Secret Manager access_secret_version call#63813
evgeniy-b wants to merge 1 commit intoapache:mainfrom
evgeniy-b:fix-secret-manager-grpc-timeout

Conversation

@evgeniy-b
Copy link

@evgeniy-b evgeniy-b commented Mar 17, 2026

Summary

_SecretManagerClient.get_secret() calls access_secret_version() with no timeout parameter, meaning the gRPC call has no deadline. If Secret Manager is slow or unreachable, the calling thread blocks indefinitely.

This is particularly problematic in the triggerer, where Secret Manager calls run in the shared sync_to_async thread pool. A single stuck call can exhaust the pool, blocking all triggers.

Changes

  • Add a 30s gRPC deadline (timeout=30) to the access_secret_version call in _SecretManagerClient.get_secret()
  • Catch DeadlineExceeded and re-raise as Python TimeoutError so callers can handle it with standard exception handling
  • Log a warning on timeout so operators can detect Secret Manager connectivity issues

The timeout kwarg sets a gRPC deadline enforced at the C level inside grpc-core. When the deadline expires, grpc-core cancels the RPC and the thread unblocks with a DeadlineExceeded error — unlike Python-level timeouts which cannot interrupt threads blocked in C code.

Why TimeoutError instead of returning None?

Returning None would mean "secret doesn't exist," which is semantically wrong for a timeout — the secret may exist, we just couldn't reach it. The secrets backend framework in Connection.get_connection_from_secrets() already catches Exception and falls through to the next backend, so re-raising is safe and preserves the distinction.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Opus 4.6

Generated-by: Claude Opus 4.6 following the guidelines

The `_SecretManagerClient.get_secret()` calls
`access_secret_version()` with no timeout, so the gRPC call has no
deadline. If Secret Manager is slow or unreachable the calling thread
blocks indefinitely. Add a 30 s gRPC deadline and re-raise
`DeadlineExceeded` as `TimeoutError`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@evgeniy-b evgeniy-b requested a review from shahar1 as a code owner March 17, 2026 12:26
@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Mar 17, 2026
@potiuk potiuk marked this pull request as draft March 17, 2026 17:37
@potiuk
Copy link
Member

potiuk commented Mar 17, 2026

@evgeniy-b This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

  • Provider tests: Failing: provider distributions tests / Compat 2.11.1:P3.10:, provider distributions tests / Compat 3.0.6:P3.10:, provider distributions tests / Compat 3.1.8:P3.10:, Non-DB tests: providers / Non-DB-prov::3.10:amazon...standard, Low dep tests: providers / All-prov:LowestDeps:14:3.10:common.messag...google. Run provider tests with breeze run pytest <provider-test-path> -xvs. See Provider tests docs.
  • Other failing CI checks: Failing: Analyze (actions). Run prek run --from-ref main locally to reproduce. See static checks docs.

What to do next:

  • The comment informs you what you need to do.
  • Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
  • There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
  • Maintainers will then proceed with a normal review.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

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

Labels

area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants