Defer GoogleSecretManagerSettingsSource client initialization to __call__#866
Draft
ecerulm wants to merge 1 commit into
Draft
Defer GoogleSecretManagerSettingsSource client initialization to __call__#866ecerulm wants to merge 1 commit into
ecerulm wants to merge 1 commit into
Conversation
- Move all GCP client setup (google_auth_default, SecretManagerServiceClient) out of __init__ and into a new _initialize_client() method called lazily from __call__. - Allow project_id to be resolved from self.current_state (populated by previous settings sources) when not explicitly provided, enabling patterns like storing the GCP project ID in env vars or init kwargs and having the GCP source pick it up automatically. - _load_env_vars() returns an empty dict during __init__ (before the client is initialized); __call__ overwrites self.env_vars with the real GoogleSecretManagerMapping before delegating to the parent. - Replace isinstance(_project_id, str) guard with an idiomatic None check. - Update tests to reflect deferred initialization: assertions on _project_id now call _initialize_client() first; the AttributeError test now expects the error at _initialize_client() time rather than __init__ time. - Add test_project_id_from_current_state to verify the new end-to-end flow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
google_auth_default,SecretManagerServiceClient) is moved out of__init__into a new_initialize_client()method, called from__call__. This avoids side-effects (network calls, credential resolution) at import/construction time.project_idfrom previous sources: Ifproject_idis not explicitly passed,_initialize_client()checksself.current_state.get('project_id')— which is populated by previously-run settings sources — before falling back togoogle_auth_default(). This enables patterns like placing the GCP project ID in environment variables or init kwargs and having the GCP source pick it up automatically._load_env_vars()guard: Returns an empty dict during__init__(before the client exists);__call__repopulatesself.env_varswith the realGoogleSecretManagerMappingbefore delegating to the parent.isinstance(_project_id, str)with_project_id is not None._initialize_client()explicitly (since resolution is now deferred); newtest_project_id_from_current_stateverifies the end-to-end flow.