Skip to content

Fix for case-insensitive alias matching in InitSettingsSource #609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Shindora
Copy link

@Shindora Shindora commented Apr 24, 2025

Issue

When using case-insensitive settings with aliases, the InitSettingsSource wasn't correctly matching field aliases in a case-insensitive manner due to not considering case sensitivity when obtaining alias names.

Changes

Modified the __init__ method of InitSettingsSource to pass the case_sensitive parameter to get_alias_names() when generating match aliases
Now properly creates two sets of aliases:

  • Canonical aliases (always case-sensitive) for the output key
  • Match aliases that respect the configured case sensitivity for matching input values

Add unittest test_init_settings_source_extra_fields_case_sensitive

@hramezani
Copy link
Member

hramezani commented Apr 24, 2025

Thanks @Shindora for the PR.

Could you please add some examples that don't work with the existing code? You also need to add tests for these changes.

@kschwab do you have time to take a look?

@hramezani
Copy link
Member

@Shindora, you've added two tests to the PR, but both pass without your change in base.py

@hramezani
Copy link
Member

@Shindora, the 'test_case_sensitive ' test passes without your change in base.py.

@hramezani
Copy link
Member

This is the only failing case

    class CaseInsensitiveSettings(BaseSettings):
        foo: str = Field(..., alias='FOO')
        model_config = SettingsConfigDict(case_sensitive=False, extra='allow')

    # Test case-insensitive with extra field
    monkeypatch.setattr(os, 'environ', value={})
    init_kwargs = {'Foo': 'foo_value', 'extra_field': 'extra_value'}
    settings = CaseInsensitiveSettings(**init_kwargs)
    assert settings.foo == 'foo_value'
    assert settings.__pydantic_extra__ == {'extra_field': 'extra_value'}

Please revert your change in test_case_sensitive

cc @kschwab

@Shindora
Copy link
Author

This is the only failing case

    class CaseInsensitiveSettings(BaseSettings):
        foo: str = Field(..., alias='FOO')
        model_config = SettingsConfigDict(case_sensitive=False, extra='allow')

    # Test case-insensitive with extra field
    monkeypatch.setattr(os, 'environ', value={})
    init_kwargs = {'Foo': 'foo_value', 'extra_field': 'extra_value'}
    settings = CaseInsensitiveSettings(**init_kwargs)
    assert settings.foo == 'foo_value'
    assert settings.__pydantic_extra__ == {'extra_field': 'extra_value'}

Please revert your change in test_case_sensitive

cc @kschwab

reverted ! I add the new test case to cover my adjustment test_init_settings_source_extra_fields_case_sensitive. Please take a look.

@hramezani
Copy link
Member

Thanks @Shindora for the update.

Actually, InitSettingsSource is similar to passing data directly to pydantic. So, I am not sure we want to support case_sensitive in InitSettingsSource. I'm afraid it breaks people's code. Especially, the amount of code change to support this case seems too much to me.

What do you think @kschwab?

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