Skip to content

Conversation

@MolloKhan
Copy link

Q A
Bug fix? no
New feature? yes
Docs? yes
License MIT

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 interface drop() 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\Store without adding more properties to the constructor
What do you think?

@carsonbot carsonbot added Status: Needs Review Feature New feature Store Issues & PRs about the AI Store component labels Nov 13, 2025
@MolloKhan
Copy link
Author

MolloKhan commented Nov 13, 2025

The second part of this PR would be to improve the SetupStoreCommand command. After creating the store, the user needs to know the just created host and update the Pinecone client config.
I'm thinking of returning the response when calling ManagedStoreInterface::setup() and adding another method that explains what to do next.
For example: "Update the Pinecone Client host property with: {$host}"

@chr-hertel chr-hertel marked this pull request as draft November 14, 2025 20:29
@chr-hertel
Copy link
Member

I'd be in favor of extending the interface here - @OskarStark @Guikingone what do you think?

@Guikingone
Copy link
Contributor

Agree with @chr-hertel 👍🏻

Copy link
Member

@chr-hertel chr-hertel left a 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

@MolloKhan
Copy link
Author

Yea, I will, unfortunately I won't be available until after the SymfonyCon :)

@MolloKhan MolloKhan force-pushed the pinecone-managed-store branch from 766ad1a to 741a750 Compare December 12, 2025 19:00
Copy link
Contributor

@OskarStark OskarStark left a 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

@MolloKhan
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New feature Status: Needs Review Store Issues & PRs about the AI Store component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants