-
Notifications
You must be signed in to change notification settings - Fork 16k
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
core[minor]: Enhance cache flexibility in BaseChatModel #17386
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you making so many indent changes? The issue only specified cache. I'd suggest you to revert them
@eyurtsev Could you please confirm whether you intend to include caching functionality in the |
We should iterate in smalls steps and match existing global cache functionality. If we want to extend the caching for streaming, we can do so, but let's do it as a separate task if it's not covered by existing global cache. |
Hi @eyurtsev I added the inline docstring and added test. Could not work on external documentation. Could anybody help me here @hasansustcse13 @Lord-Haji |
InMemoryCache, | ||
SQLAlchemyCache, | ||
) | ||
from langchain.globals import get_llm_cache, set_llm_cache | ||
|
||
|
||
class MockCache(BaseCache): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alhridoy This is not the correct BaseCache interface.
Do you want to try and fix up the code? You can use InMemoryCache implementation to test things out.
At the moment, the included unit tests have an number of errors in them, so testing code does not work
@@ -15,12 +16,27 @@ | |||
from sqlalchemy.orm import Session | |||
|
|||
from langchain.cache import ( | |||
BaseCache, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BaseCache
is not available from langchain.cache, instead it's in langchain_core.cache
I'm going to commandeer to we can land this |
Hi @eyurtsev Give me a few days. I'll try it first. Thanks for your patience and guidance. |
@alhridoy just saw your message -- i made the changes already. I'm going to merge for now if tests pass, feel free to review the changes. And there are a lot of contribution opportunities so we'll appreciate any help :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…#17386) - **Description:** Enhanced the `BaseChatModel` to support an `Optional[Union[bool, BaseCache]]` type for the `cache` attribute, allowing for both boolean flags and custom cache implementations. Implemented logic within chat model methods to utilize the provided custom cache implementation effectively. This change aims to provide more flexibility in caching strategies for chat models. - **Issue:** Implements enhancement request langchain-ai#17242. - **Dependencies:** No additional dependencies required for this change. --------- Co-authored-by: Eugene Yurtsev <[email protected]>
…#17386) - **Description:** Enhanced the `BaseChatModel` to support an `Optional[Union[bool, BaseCache]]` type for the `cache` attribute, allowing for both boolean flags and custom cache implementations. Implemented logic within chat model methods to utilize the provided custom cache implementation effectively. This change aims to provide more flexibility in caching strategies for chat models. - **Issue:** Implements enhancement request langchain-ai#17242. - **Dependencies:** No additional dependencies required for this change. --------- Co-authored-by: Eugene Yurtsev <[email protected]>
- **Description:** Enhanced the `BaseChatModel` to support an `Optional[Union[bool, BaseCache]]` type for the `cache` attribute, allowing for both boolean flags and custom cache implementations. Implemented logic within chat model methods to utilize the provided custom cache implementation effectively. This change aims to provide more flexibility in caching strategies for chat models. - **Issue:** Implements enhancement request #17242. - **Dependencies:** No additional dependencies required for this change. --------- Co-authored-by: Eugene Yurtsev <[email protected]>
Please title your PR "core: Enhance cache flexibility in BaseChatModel".
BaseChatModel
to support anOptional[Union[bool, BaseCache]]
type for thecache
attribute, allowing for both boolean flags and custom cache implementations. Implemented logic within chat model methods to utilize the provided custom cache implementation effectively. This change aims to provide more flexibility in caching strategies for chat models.