Skip to content

Conversation

@sunyuhan1998
Copy link
Contributor

In the saveAll method of JdbcChatMemoryRepository, all historical messages of the current conversation are first deleted, and then the latest list of messages is saved. This operation should be transactional. This PR adds transaction support for it.

@markpollack markpollack self-assigned this May 14, 2025
@markpollack
Copy link
Member

This approach won't enable participation in a surrounding outer scope transaction, I'll work on a solution.

@quaff
Copy link
Contributor

quaff commented May 15, 2025

This approach won't enable participation in a surrounding outer scope transaction, I'll work on a solution.

Indeed, but do we really need participating in outer scope transaction here?

@sunyuhan1998
Copy link
Contributor Author

This approach won't enable participation in a surrounding outer scope transaction, I'll work on a solution.

You're right; this approach indeed cannot participate in outer scope transaction. However, it seems that ChatMemoryRepository is inherently designed with ChatMemory in mind. In such a scenario, do we really have an "outer scope transaction" that needs to be involved?

@markpollack
Copy link
Member

markpollack commented May 15, 2025

Who knows what people will do in the wild, it is a tiny change to the PR.

@markpollack
Copy link
Member

Thanks @sunyuhan1998 merged in 0e1bb52

@markpollack markpollack added this to the 1.0.0 milestone May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants