langchain-ai / langchain-postgres

LangChain abstractions backed by Postgres Backend
MIT License
66 stars 22 forks source link

PostgresChatMessageHistory API break the usage of LCEL and Langserv #36

Open pprados opened 2 months ago

pprados commented 2 months ago

The constructor of PostgresChatMessageHistory accepts only sync or async connection, and not an Engine.

First, the connection was open in get_session_history, and pending during the life cycle of RunnableWithMessageHistory.

So, it's impossible to use it in a "singleton" approach.

With langserv, you must declare the API with

add_routes(
    app,
    chain,
    path="/",
)

You must have a global variable chain.

chat_chain = RunnableWithMessageHistory(
    _context_and_question | retriever_chain,
    get_session_history=get_session_history, # <-- here
    input_messages_key="question",
    history_messages_key="chat_history",
)

The get_session_history cannot be async. The PostgresChatMessageHistory.async_connection() can not be used.

PostgresChatMessageHistory must be used if the singleton form, to be use with langserv. The engine must be set, without connection.

eyurtsev commented 2 months ago

I'm hoping to avoid using too much SQLAlchemy. Would a psycopg pool object work for your use case?

pprados commented 1 month ago

Yes. The idea is to use the same approach as for the other integrations with sqlalchemy, exploiting the connection pool and sharing sessions if possible.

the vectorstore proposes

        connection: Union[None, DBConnection, Engine, AsyncEngine, str] = None,

I think this is preferable and usable.

@eyurtsev , I can propose a PR for that, I can propose a PR for this, but I don't want it to remain on hold for several weeks, as with my other PRs.

pprados commented 1 month ago

@eyurtsev I propose a PR to resolve this problem.

pprados commented 1 month ago

A patch is being applied here