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

Extend DynamoDBChatMessageHistory to support composite keys #9896

Merged
merged 2 commits into from
Sep 3, 2023
Merged

Extend DynamoDBChatMessageHistory to support composite keys #9896

merged 2 commits into from
Sep 3, 2023

Conversation

joshualwhite
Copy link
Contributor

  • Description: Adds two optional parameters to the DynamoDBChatMessageHistory class to enable users to pass in a name for their PrimaryKey, or a Key object itself to enable the use of composite keys, a common DynamoDB paradigm.

AWS DynamoDB Key docs

  • Issue: N/A
  • Dependencies: N/A
  • Twitter handle: N/A

@vercel
Copy link

vercel bot commented Aug 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Aug 29, 2023 10:40pm
langchain-deprecated ⬜️ Ignored (Inspect) Visit Preview Aug 29, 2023 10:40pm

@dosubot dosubot bot added Ɑ: memory Related to memory module 🤖:improvement Medium size change to existing code to handle new use-cases labels Aug 29, 2023
@@ -28,10 +27,21 @@ class DynamoDBChatMessageHistory(BaseChatMessageHistory):
is optional and useful for test purposes, like using Localstack.
If you plan to use AWS cloud service, you normally don't have to
worry about setting the endpoint_url.
primary_key_name: name of the primary key of the DynamoDB table. This argument
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we specify that only one of primary_key_name or key should be passed in

@hwchase17 hwchase17 merged commit bc8ccee into langchain-ai:master Sep 3, 2023
26 checks passed
@@ -84,6 +93,6 @@ def clear(self) -> None:
from botocore.exceptions import ClientError

try:
self.table.delete_item(Key={"SessionId": self.session_id})
self.table.delete_item(self.key)
Copy link
Contributor

@Nantero1 Nantero1 Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeError: delete_item() only accepts keyword arguments.

IMHO this should be self.table.delete_item(Key=self.key)
@joshualwhite

baskaryan pushed a commit that referenced this pull request Sep 11, 2023
)

**Description**: 
Fixed a bug introduced in version 0.0.281 in
`DynamoDBChatMessageHistory` where `self.table.delete_item(self.key)`
produced a TypeError: `TypeError: delete_item() only accepts keyword
arguments`. Updated the method call to
`self.table.delete_item(Key=self.key)` to resolve this issue.

Please see also [the official AWS
documentation](https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb/table/delete_item.html#)
on this **delete_item** method - only `**kwargs` are accepted.

See also the PR, which introduced this bug:
#9896 (comment)

Please merge this, I rely on this delete dynamodb item functionality
(because of GDPR considerations).

**Dependencies**: 
None

**Tag maintainer**: 
@hwchase17 @joshualwhite 

**Twitter handle**: 
[@BenjaminLinnik](https://twitter.com/BenjaminLinnik)
Co-authored-by: Benjamin Linnik <Benjamin@Linnik-IT.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases Ɑ: memory Related to memory module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants