-
Notifications
You must be signed in to change notification settings - Fork 725
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
Fix: defined default var for langchain cache path to enable custom path (issue 987) #982
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks for your code, I've added two comments.
@@ -420,10 +424,7 @@ def load_settings(): | |||
"Your config file is outdated. Please delete it and restart the app to regenerate it." | |||
) | |||
|
|||
lc_cache_path = os.path.join(config_dir, ".langchain.db") | |||
|
|||
project_settings = ProjectSettings( |
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.
I believe you're missing something to change the project_config["lc_cache_path"]
value, like:
project_settings = ProjectSettings( | |
project_config["lc_cache_path"] = project_config.get("lc_cache_path", lc_cache_path) | |
project_settings = ProjectSettings( |
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.
the way i understand it and tested it locally, possible custom path will be picked up by toml_dict
module here:
project_config = toml_dict.get("project", {})
or will default to default value as proposed by my PR here
# default to "{config_dir}/.langchain.db"
lc_cache_path: Optional[str] = os.path.join(config_dir, ".langchain.db")
@@ -19,7 +19,7 @@ def init_lc_cache(): | |||
if config.project.lc_cache_path is not None: | |||
set_llm_cache(SQLiteCache(database_path=config.project.lc_cache_path)) | |||
|
|||
if not os.path.exists(config.project.lc_cache_path): | |||
if os.path.exists(config.project.lc_cache_path): |
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.
I'm not sure to understand why you've removed this?
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.
it is because previous line st_llm_cache
generates sqlite db file at path
set_llm_cache(SQLiteCache(database_path=config.project.lc_cache_path))
consequently in next line we want to inform cache was generated by testing if path exists and not the opposite. I hope I was clear - was trying not to use too many "if", "not" in the explaination
When defining
lc_cache_path
and enabling cache in config:TypeError is thrown: