Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pydantic_settings/sources/providers/gcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ def __getitem__(self, key: str) -> str | None:
name=self._secret_version_path(key)
).payload.data.decode('UTF-8')
except Exception:
raise KeyError(key)
# If we can't access the secret, we return None
self._loaded_secrets[key] = None
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change. Not sure whether this is the correct behaviour.

Probably we can add this behaviour in V2 by a flag that is disabled by default.

@ezwiefel what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dug through all the code paths I could find that would get to that point and didn't see where throwing the KeyError would be useful. I certainly could be missing something.

Copy link
Member

Choose a reason for hiding this comment

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

agree KeyError is not useful. let's merge the PR and see.


return self._loaded_secrets[key]

Expand Down
36 changes: 34 additions & 2 deletions tests/test_source_gcp_secret_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ def test_secret_manager_mapping_getitem_access_error(self, secret_manager_mappin
side_effect=Exception('Access denied')
)

with pytest.raises(KeyError):
_ = secret_manager_mapping['test-secret']
assert secret_manager_mapping['test-secret'] is None

def test_secret_manager_mapping_iter(self, secret_manager_mapping):
assert list(secret_manager_mapping) == ['test-secret']
Expand Down Expand Up @@ -210,3 +209,36 @@ def settings_customise_sources(

with pytest.raises(ValidationError):
_ = Settings()

def test_pydantic_base_settings_with_default_value(self, mock_secret_client):
class Settings(BaseSettings):
my_field: str | None = Field(default='foo')

@classmethod
def settings_customise_sources(
cls,
settings_cls: type[BaseSettings],
init_settings: PydanticBaseSettingsSource,
env_settings: PydanticBaseSettingsSource,
dotenv_settings: PydanticBaseSettingsSource,
file_secret_settings: PydanticBaseSettingsSource,
) -> tuple[PydanticBaseSettingsSource, ...]:
google_secret_manager_settings = GoogleSecretManagerSettingsSource(
settings_cls, secret_client=mock_secret_client
)
return (
init_settings,
env_settings,
dotenv_settings,
file_secret_settings,
google_secret_manager_settings,
)

settings = Settings()
assert settings.my_field == 'foo'

def test_secret_manager_mapping_list_secrets_error(self, secret_manager_mapping, mocker):
secret_manager_mapping._secret_client.list_secrets = mocker.Mock(side_effect=Exception('Permission denied'))

with pytest.raises(Exception, match='Permission denied'):
_ = secret_manager_mapping._secret_names