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

Fix: defined default var for langchain cache path to enable custom path (issue 987) #982

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

necjamahim
Copy link

When defining lc_cache_path and enabling cache in config:

# Enable third parties caching (e.g LangChain cache)
cache = true
lc_cache_path = './.custom-cache-path.db'

TypeError is thrown:

TypeError: chainlit.config.ProjectSettings() got multiple values for keyword argument 'lc_cache_path'

@necjamahim necjamahim changed the title defined default var for langchain cache path to enable custom path Fix: defined default var for langchain cache path to enable custom path (issue 987) May 13, 2024
Copy link
Collaborator

@tpatel tpatel left a 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(
Copy link
Collaborator

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:

Suggested change
project_settings = ProjectSettings(
project_config["lc_cache_path"] = project_config.get("lc_cache_path", lc_cache_path)
project_settings = ProjectSettings(

Copy link
Author

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):
Copy link
Collaborator

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?

Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants