-
-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Zep cloud setup #1760
Zep cloud setup #1760
Conversation
@@ -22,6 +22,7 @@ | |||
"@datastax/astra-db-ts": "^0.1.2", | |||
"@dqbd/tiktoken": "^1.0.7", | |||
"@elastic/elasticsearch": "^8.9.0", | |||
"@getzep/zep-cloud": "npm:@getzep/zep-js@next", |
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.
Note: We are aliasing a different version of our ts sdk (tagged with next
) as we currently only have support for Zep Cloud API in Release Candidate of our ts sdk
@@ -0,0 +1,181 @@ | |||
import { IMessage, INode, INodeData, INodeParams, MemoryMethods, MessageType } from '../../../src/Interface' |
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.
instead of creating a new node, what do u think if we just add a new options
field to the ZepMemory
that allow users to select OpenSource or Cloud, then according to selection, use the respective library for init
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 think this would be doable, but I have some reservations.
Our interfaces are not the same for open source and cloud sdk. For memory, for example, we don't offer the ability to specify memoryType
on Open Source version, whereas this is one of the new things we have introduced with Zep Cloud. The same applies to vector stores, in cloud fro example we don't accept embeddings and embedding dimensions any more.
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.
So we would have to have a lot of additional if blocks in the nodes.
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.
Oh okay i thought the difference is only the initialization. In that case, make sense to have separate nodes
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.
Cool! Marked it as ready for review then!
@paul-paliychuk some linting issues if you can do |
@HenryHengZJ My bad, just ran the linter-fixer |
|
||
constructor() { | ||
this.label = 'Zep Collection - Cloud' | ||
this.name = 'Zep Collection - Cloud' |
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.
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 @paul-paliychuk !
No description provided.