feat: Add api_key_env_var to Model, pass in kwargs to langchain initializer #1142
Conversation
Documentation preview |
There was a problem hiding this comment.
Thank you Tim,
I suggest that we do not change init_llm_model and update kwargs in llmrails.py to pick up the value of env var (if set). The only test that requires fix is test_llm_rails_config.py.
A soft reset and deleting the files except rails/llm/config.py and rails/llm/llmrails.py, and examples/configs/content_safety_api_key should do the trick:
git reset --soft 90760ab16a9dc659ad0580348af1a2eeda0a7149
git restore --staged .
git restore nemoguardrails/llm/models/initializer.py \
nemoguardrails/llm/models/langchain_initializer.py \
nemoguardrails/llm/providers/providers.py \
tests/llm_providers/test_langchain_initialization_methods.py \
tests/test_bug_5.py \
tests/test_rails_llm_config.py \
tests/v2_x/test_passthroug_mode.pyand then we should make the changes to llmrails.py and config.py 👍🏻
What do you think?
e2137b9 to
52d4ad9
Compare
Sounds great, made the necessary changes and added tests to cover the environment variable checks in RailsConfig |
There was a problem hiding this comment.
Thanks @tgasser-nv, the PR looks great overall! Thank you for adding comprehensive tests 👍🏻
I noticed two minor issues, so I went ahead and pushed some minor fixes, updated the tests, and added two integration tests. Please let me know if you are OK with those changes.
p.s. test_langchain_integration.py is skipped on CI
This reverts commit 1fec637.
- Added tests for initializing chat and text models using a custom API key environment variable (`NG_OPENAI_API_KEY`).
Update test assertions to match actual init_chat_model behavior where kwargs are unpacked rather than passed as a named parameter.
- Changed `NG_OPENAI_API_KEY` to `DUMMY_OPENAI_API_KEY`. - Changed `NVIDIA_API_KEY` to `DUMMY_NVIDIA_API_KEY`. - Updated test cases to use dummy key names to prevent conflicts with user defined environment variables when running tests locally.
6d0bc2b to
69e1f9e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1142 +/- ##
========================================
Coverage 68.00% 68.01%
========================================
Files 161 161
Lines 15793 15805 +12
========================================
+ Hits 10740 10749 +9
- Misses 5053 5056 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Related Issue(s)
Checklist