-
-
Notifications
You must be signed in to change notification settings - Fork 142
[Store] Add Pinecone ManagedStore #865
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
base: main
Are you sure you want to change the base?
Conversation
|
The second part of this PR would be to improve the |
|
I'd be in favor of extending the interface here - @OskarStark @Guikingone what do you think? |
|
Agree with @chr-hertel 👍🏻 |
chr-hertel
left a comment
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.
Please tackle your proposed changes to the interface with this PR
|
Yea, I will, unfortunately I won't be available until after the SymfonyCon :) |
766ad1a to
741a750
Compare
OskarStark
left a comment
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 agree on the options for drop(), should be a separate PR, but the indexable should be configurable on the store and therefore be set in the constructor IMHO
Yes, I think you're right. To set up a Pinecone server, you only need to define an index name (along with the API key). Upon that, you get a host value that you need to attach to the Pinecone client config. So, in short, those two values should be bound together to avoid unexpected errors |
This is a draft PR for managing Pinecone store.
Instead of having a property for each value (indexName, dimension, etc), I wanted to send them as
$options, so the service can create different kinds of Pinecone index, but it is not possible without modifying the interfacedrop()method, which requires knowing what index you want to remove.If we update the interface, I could merge this class into
Symfony\AI\Store\Bridge\Pinecone\Storewithout adding more properties to the constructorWhat do you think?