Skip to content

[BUG]: Ignore extra env vars in Settings to prevent ValidationError#6535

Open
basnijholt wants to merge 3 commits intochroma-core:mainfrom
basnijholt:fix-settings-extra-ignore
Open

[BUG]: Ignore extra env vars in Settings to prevent ValidationError#6535
basnijholt wants to merge 3 commits intochroma-core:mainfrom
basnijholt:fix-settings-extra-ignore

Conversation

@basnijholt
Copy link
Copy Markdown
Contributor

@basnijholt basnijholt commented Mar 3, 2026 •
edited
Loading

Follow-up to #6356 — that PR switched Settings from pydantic v1 BaseSettings to pydantic_settings.BaseSettings (v2), but didn't set extra="ignore".

pydantic_settings.BaseSettings v2 defaults to extra="forbid", so any environment variable or .env key that doesn't match a Settings field (e.g. OPENAI_API_KEY, GEMINI_API_KEY) raises a ValidationError at import time:

ValidationError: 5 validation errors for Settings
gemini_api_key
  Extra inputs are not permitted [type=extra_forbidden, ...]
openai_api_key
  Extra inputs are not permitted [type=extra_forbidden, ...]

The old pydantic v1 BaseSettings silently ignored unrecognized env vars. Adding "extra": "ignore" to model_config restores that behavior.

@basnijholt
[BUG]: Ignore extra env vars in Settings to prevent ValidationError
ed2dff5 
`pydantic_settings.BaseSettings` defaults to `extra="forbid"`, which
means any environment variable or `.env` key that doesn't match a
`Settings` field raises a `ValidationError` at import time.

This is a follow-up to chroma-core#6356 which switched from pydantic v1
`BaseSettings` to pydantic-settings v2. The old v1 `BaseSettings`
silently ignored unrecognized env vars, but v2 rejects them by default.

Adding `"extra": "ignore"` restores the previous tolerance so users
with unrelated env vars (e.g. `OPENAI_API_KEY`, `GEMINI_API_KEY`) in
their environment or `.env` file can import chromadb without errors.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

propel-code-bot[bot]

This comment was marked as outdated.

Sign in to view

@propel-code-bot
Copy link
Copy Markdown
Contributor

propel-code-bot bot commented Mar 3, 2026 •
edited
Loading

It also adds regression tests to confirm extra environment variables and initialization kwargs are ignored and not exposed as attributes.

Possible Issues

• Ignoring extras could mask typos in environment variable names, which may delay detection of misconfiguration.

This summary was automatically generated by @propel-code-bot

@basnijholt basnijholt mentioned this pull request Mar 3, 2026
Merged
@kylediaz
Copy link
Copy Markdown
Contributor

kylediaz commented Mar 3, 2026

#6534

propel-code-bot[bot]

This comment was marked as outdated.

Sign in to view

@kylediaz
Copy link
Copy Markdown
Contributor

kylediaz commented Mar 4, 2026

Yeah, I have a hangup for your test because you're using os.chdir and writing to the filesystem. There's an easier way to test this using pytest's tools. I wouldn't mind if you just copy the tests from this PR, and we can merge this PR

#6534

propel-code-bot[bot]
propel-code-bot bot reviewed Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@propel-code-bot propel-code-bot bot left a comment

Choose a reason for hiding this comment

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

No issues found; changes appear aligned with desired behavior and tested.

Status: No Issues Found | Risk: Low

Review Details

📁 2 files reviewed | 💬 0 comments

@kylediaz kylediaz self-requested a review March 5, 2026 01:50
@basnijholt
Copy link
Copy Markdown
Contributor Author

basnijholt commented Mar 5, 2026

@kylediaz I did that! 😄

@kylediaz
Copy link
Copy Markdown
Contributor

kylediaz commented Mar 5, 2026

We added multi-cloud support for multi-node Chroma, so our CI is broken for external contributors. I need to get a repo admin to bypass the merge requirements. Sorry about this!

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

Reviewers

@propel-code-bot propel-code-bot[bot] propel-code-bot[bot] left review comments

@kylediaz kylediaz kylediaz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@basnijholt @kylediaz