Repository navigation
fix(python): treat blank API keys as unset and read OPENAI_ORGANIZATION - #446
Mohammed Alkindi (MohammedAlkindi) wants to merge 3 commits into
Conversation
A blank OPENAI_API_KEY, as left by copying .env.example and filling in only the Azure section, chose OpenAI and sent an empty bearer token. Keys are now chosen by value, as typescript/src/model.ts does.
The Python port read only OPENAI_ORG, while the README and the TypeScript port use OPENAI_ORGANIZATION. OPENAI_ORG is still read as a fallback.
robgruen
left a comment
There was a problem hiding this comment.
Please see my comment about auth mode. If we don't want to implement that in this PR than the change I would accept is just the fallback for the OPENAI_ORGANIZATION -> OPENAI_ORG variable.
|
I worry about this change breaking existing TypeChat users that rely on existing behavior. It is possible to use keyless authentication and/or supply credentials after creating the model instance. A cleaner fix would be to add support for proper automatic credential acquisition/refresh. So, if we are going to make a breaking change let's move everyone forward to better auth handling. As is, this change isn't breaking so we would be changing behavior with no interface change and so it could silently cause issues. We should avoid that if possible. |
Reverts the blank-key change from 94e33e3 so a present but empty OPENAI_API_KEY still selects OpenAI, as it did before. Callers that supply credentials later or rely on the current selection keep working. This PR now only adds the OPENAI_ORGANIZATION fallback.
|
Agreed, the blank-key change could break callers silently. f61c6bf reverts it, so model selection behaves exactly as on main, and the PR is now only the |
The Python
create_language_modelread the organization header only fromOPENAI_ORG, while both example READMEs andtypescript/src/model.tsuseOPENAI_ORGANIZATION. Setting the documented variable therefore sent noOpenAI-Organizationheader.It now reads
OPENAI_ORGANIZATIONfirst and falls back toOPENAI_ORG, so existing setups keep working.Three tests in
python/tests/test_create_language_model.pycover the new variable, the fallback, and the precedence when both are set.pytestinpython/: 25 passed. pyright 1.1.358 reports no errors in the two changed files.