fix(config): read OPENAI_API_KEY in the load path, not from_persisted#15
Open
flobernd wants to merge 1 commit into
Open
fix(config): read OPENAI_API_KEY in the load path, not from_persisted#15flobernd wants to merge 1 commit into
flobernd wants to merge 1 commit into
Conversation
Config::from_persisted read OPENAI_API_KEY directly inside parse_upstreams to seed any keyless upstreams entry. That made every caller of from_persisted ambient-environment-sensitive, including its own unit tests: whichever value OPENAI_API_KEY happened to hold in the process at call time silently leaked into the result, a source of test flakiness and surprising behavior for any caller building a Config straight from a PersistedConfig value. Move the seeding into a new seed_upstream_api_keys_from_env step, applied only in the load path (from_env_and_file) after apply_env_overrides and before from_persisted. from_persisted is now a pure function of its input; the load path's observable startup behavior (a keyless upstreams entry inherits OPENAI_API_KEY, an explicit api_key is never overridden) is unchanged. Closes local-inference-lab#34
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
Config::from_persistedno longer readsOPENAI_API_KEY. The per-entry seeding of keylessupstreamsentries moves to the load path (from_env_and_file), so config parsing is pure and no longer ambient-environment-sensitive, while production startup behavior is unchanged. Explicitapi_keyvalues are never overridden.Testing
Full
cargo testgreen (742 unit tests plus all integration suites, 0 failed),cargo fmt --checkclean,cargo clippy --all-targetszero warnings. New tests: a purity test assertingfrom_persistedignores the env var, and an end-to-end load-path test asserting the seed still applies at startup; the existing seeding tests were adapted to the load path.Closes local-inference-lab#34.
Notes
Stacked on local-inference-lab#36's branch; retarget or rebase once that merges.