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.

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

openai[patch]: change in client_params via config updates the client #23685

Closed
wants to merge 11 commits into from

Conversation

spike-spiegel-21
Copy link
Contributor

Description: Changes in client parameters can be done using ConfigurableFields
Issue: note: There might be more related issues.
fixes: #23381 (langchain-ai/langserve#693) (langchain-ai/langserve#652)
Twitter handle: @mynksol

@efriis efriis added the partner label Jun 30, 2024
@efriis efriis self-assigned this Jun 30, 2024
Copy link

vercel bot commented Jun 30, 2024

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

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Jul 8, 2024 6:07pm

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. 🔌: openai Primarily related to OpenAI integrations 🤖:improvement Medium size change to existing code to handle new use-cases labels Jun 30, 2024
@spike-spiegel-21 spike-spiegel-21 changed the title openai[patch]: change in client_params updates the client using config openai[patch]: change in client_params via config updates the client Jun 30, 2024
@quadcube
Copy link

quadcube commented Jul 1, 2024

@spike-spiegel-21 hey there! thank you for working on the patch.
I have tested your fix (mainly removal of the 'values.get("client")' and it's async equivalent), it works fine on my end and fixed the problem

@quadcube
Copy link

quadcube commented Jul 1, 2024

On further testing, this works for regular chain invoke but it seems that it does not work for agents.
May be related to these:
langchain-ai/langserve#314
#17555

@spike-spiegel-21
Copy link
Contributor Author

Noted @quadcube This PR was intended to solve the issue only for the langchain_openai chat model. I will raise a separate PR for:
langchain-ai/langserve#314
#17555

@quadcube
Copy link

quadcube commented Jul 1, 2024

@spike-spiegel-21 if it's helpful, I can open an issue with the code to reproduce the problem for the configurable agent.

@spike-spiegel-21
Copy link
Contributor Author

@spike-spiegel-21 if it's helpful, I can open an issue with the code to reproduce the problem for the configurable agent.

@quadcube Thank You!

@spike-spiegel-21
Copy link
Contributor Author

@baskaryan @efriis Please have a look. I came up with the best resolution after debugging

Copy link
Member

@efriis efriis left a comment

Choose a reason for hiding this comment

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

this is currently a breaking change for a few cases (including the ones that are failing tests, as well as anyone manually passing in client or async_client)

the case you want to fix is just adding configurability to the openai_api_key?

going to mark this as a draft while we sort out the best way to do this

@efriis efriis marked this pull request as draft July 12, 2024 23:07
@efriis
Copy link
Member

efriis commented Aug 24, 2024

closing as stale - let me know if you have thoughts above!

@efriis efriis closed this Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases 🔌: openai Primarily related to OpenAI integrations partner size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BaseChatModel.agenerate_prompt() not passing kwargs correctly to BaseChatModel.agenerate()
3 participants