Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mask API key for Tongyi ChatModel and LLM #14170

Closed
wants to merge 27 commits into from
Closed

Mask API key for Tongyi ChatModel and LLM #14170

wants to merge 27 commits into from

Conversation

fadilparves
Copy link

@fadilparves fadilparves commented Dec 2, 2023 •
edited
Loading

@fadilparves
added Union import and SecretStr import to tongyi.py
875be81
@fadilparves
added function that will convert the raw string to SecretStr
3538e68
@fadilparves
added property method for getting the api key from env and converting…
c9b1233 
… it to SecretStr
@fadilparves
fix type from dashcope to dashscope_api_key
b9addc0
@fadilparves
added get_secret_value() when calling the api_key for tongyi client
fc4cc16
@fadilparves
added the test file for tyongi chat model
c4bb303
@fadilparves
added dashscope to the extended_testing in pyproject.toml to check if…
d43867b 
… the package is not installed then skip the test
@fadilparves
adjust formatting of code
492d008
@fadilparves
adjusting the var name for property function
31460cd
@fadilparves
fix the firt test case by adding _to_secret in root validator
ade1f4f
@fadilparves
all 3 test is passed
6eab833
@fadilparves
added dashscope as optional deps in pyproject.toml
d09ba4c
@fadilparves
updated the poetry to add dashscope and also the toml file to make da…
283064a 
…shscope as the extended testing deps
@fadilparves
adding file poetry.lock and pyproject.toml missed it in the last commit
fb2b490
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 2, 2023
@vercel Vercel
Copy link

vercel bot commented Dec 2, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2023 7:00pm

@dosubot dosubot bot added Ɑ: models Related to LLMs or chat model modules 🤖:improvement Medium size change to existing code to handle new use-cases labels Dec 2, 2023
@fadilparves
added test for tongyi llm module
93b1c32
@fadilparves
added the 3 test cases for tongyi chat models
efffaf8
@fadilparves
fix the issue where the api_key could be none hence calling get_secre…
ae35d6e 
…t_value on None would be wrong and causing python to throw error, hence add if else checker if None then do not ask for the get_secret_value
@fadilparves
added the to secret function
5fbfeaa
@fadilparves
all 3 test is passing for tongyi llm after adding the SecretStr
c4d31b2
@fadilparves
fixed the passing of api key in kwargs to the dashscope generation ca…
9bcc1ed 
…ll function to run the method and get the response and all 3 test is also passing for checking if the key is SecretStr
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Dec 3, 2023
@fadilparves fadilparves changed the title Mask API key for Tongyi ChatModel Mask API key for Tongyi ChatModel and LLM Dec 3, 2023
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Dec 3, 2023
@dosubot dosubot bot removed the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Dec 3, 2023
@eyurtsev eyurtsev self-requested a review December 5, 2023 16:45
@fadilparves
Copy link
Author

Hi @eyurtsev , @hwchase17 , is there something that needs to be fixed on my site to merge this PR? Thank you

@hwchase17 hwchase17 closed this Jan 30, 2024
@baskaryan baskaryan reopened this Jan 30, 2024
@baskaryan
Copy link
Collaborator

resolved in #15102

@baskaryan baskaryan closed this Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baskaryan baskaryan baskaryan approved these changes

@eyurtsev eyurtsev Awaiting requested review from eyurtsev

Assignees

@eyurtsev eyurtsev

Labels
🤖:improvement Medium size change to existing code to handle new use-cases Ɑ: models Related to LLMs or chat model modules size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fadilparves @baskaryan @eyurtsev @hwchase17