Skip to content

Conversation

@tazlin
Copy link

@tazlin tazlin commented Aug 15, 2025

  • Support for AliasChoices objects in validation_alias, yielding the first choice as the preferred alias.
    • The first, and presumably preferred alias, is guaranteed by pydantic to be the first element of the choices list. Assuming it is in fact a string, this presumably would be a valid default value for most use cases.
    • I considered, and dismissed, the idea of seeking the first instance of a str, but I suspect that that may lead to surprising behavior for existing users of the library. For example, if the first and common/primary alias is an AliasPath, then emitting a secondary alias might end up being unexpected or undesired. As resolving AliasPaths is probably outside of the scope of settings-doc due to the inherent complexity, and existing users likely accept this limitation, I think it is preferable to only emit a value when the first element is a str.
  • Adds new test cases and fixtures for AliasChoices and AliasPath combinations to ensure intended behavior and continued error logging in the case that an AliasPath is the first member of the AliasChoice list.
    • Changes the existing validation_alias for ValidationAliasChoicesSettings.logging_level to be consistent with the pattern in the testing objects for other validation_alias, especially the parametrized fixtures.
  • Adjusted the relevant error message to print the causing object and field, which may help in conveying to users of the library what is causing the message and highlights which objects are not supported for aliases.

This PR is made with full ignorance to what the exact reasons were to not support AliasChoices in the first place. I couldn't come up with a compelling reason why, but I recognize there may be good cause for not supporting it. It is fairly obvious that there was a conscious reason at the time, considering lack of support is explicitly tested for. In the case I've overlooked something in either the code submitted or my underlying reasoning for the PR, please feel free to let me know and I'll be happy to address.

@tazlin tazlin force-pushed the support-alias-choices branch from 25e898f to 10fa013 Compare August 15, 2025 20:18
- Support for `AliasChoices` objects in `validation_alias`, yielding the first choice as the preferred alias.

- Adds new test cases and fixtures for `AliasChoices` and `AliasPath` combinations to ensure intended behavior and continued error logging in the case that an `AliasPath` is the first member of the `AliasChoice` list.
@tazlin tazlin force-pushed the support-alias-choices branch from 10fa013 to 7ea9427 Compare August 15, 2025 20:23
@codecov
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1d885b0) to head (7ea9427).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #50   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          187       189    +2     
  Branches        52        53    +1     
=========================================
+ Hits           187       189    +2     
Flag Coverage Δ
integration_tests 80.42% <25.00%> (-0.87%) ⬇️
total 100.00% <100.00%> (ø)
unit_tests 74.60% <100.00%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tazlin
Copy link
Author

tazlin commented Aug 27, 2025

If there is anything I can do to improve the PR for review, or if its just dead on arrival, feel free to let me know. I understand that you likely have other priorities. I'm only bumping for visibility - feel no pressure beyond a gentle nudge. I will happily maintain a fork for my purposes until this is merged.

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.

1 participant