Skip to content

[Store] Add support for Redis #269

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Store] Add support for Redis #269

wants to merge 1 commit into from
+869 −1

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Aug 6, 2025 •
edited
Loading

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

I continue to bench different stores, and the next one is Redis

$ php rag/redis.php                                                                                                                   
Here are some movies that fit the theme of technology:                                                                                                                                      
                                                                                                                                                                                            
1. **The Matrix**                                                                                                                                                                           
   - **Description**: A hacker discovers the world he lives in is a simulated reality and joins a rebellion to overthrow its controllers.                                                   
   - **Director**: The Wachowskis                                                                                                                                                           
                                                                                                                                                                                            
2. **Interstellar**                                                                                                                                                                         
   - **Description**: A team of explorers travel through a wormhole in space in an attempt to ensure humanity's survival.                                                                   
   - **Director**: Christopher Nolan                                                                                                                                                        
                                                                                                                                                                                            
3. **Jurassic Park**                                                                                                                                                                        
   - **Description**: During a preview tour, a theme park suffers a major power breakdown that allows its cloned dinosaur exhibits to run amok.                                             
   - **Director**: Steven Spielberg

@carsonbot carsonbot added Feature New feature Store Issues & PRs about the AI Store component Status: Needs Review labels Aug 6, 2025
OskarStark
OskarStark reviewed Aug 6, 2025
View reviewed changes
src/ai-bundle/config/options.php
->useAttributeAsKey('name')
->arrayPrototype()
->children()
->variableNode('connection_parameters')
Copy link
Contributor

Choose a reason for hiding this comment

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

what about going with a DSN here instead? For Symfony cache we use REDIS_URL etc. this way we can just reuse an existing Redis connection

Copy link
Member Author

Choose a reason for hiding this comment

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

Would be nice indeed.

But Symfony cache has a huge amount of code to parse the the DSN.

I don't want to bloat the the store for now

Copy link
Member

Choose a reason for hiding this comment

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

It's good that the RedisStore only takes an instance of Redis. It is not its responsibility to instantiate the Redis client.
The bundle configuration should accept a redis service name, which could be created directly in the application or with SncRedisBundle (which accepts dsn in config).

Copy link
Member Author

Choose a reason for hiding this comment

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

I update the config to support either

  • An array of parameter that is passed to the Redis constructor
  • A service id that points to an instance of Redis

@chr-hertel
Copy link
Member

Could you bring in a rag example as well, please? redis cloud or docker compose maybe

@OskarStark
Copy link
Contributor

docker compose if possible please 🙏

@lyrixx
Copy link
Member Author

lyrixx commented Aug 12, 2025

I added a rag example, with docker compose. I also updated the PR description

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

@GromNaN GromNaN GromNaN left review comments

@OskarStark OskarStark OskarStark left review comments

@chr-hertel chr-hertel Awaiting requested review from chr-hertel chr-hertel is a code owner

@Nyholm Nyholm Awaiting requested review from Nyholm Nyholm is a code owner

Assignees
No one assigned
Labels
Feature New feature Status: Needs Review Store Issues & PRs about the AI Store component
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lyrixx @chr-hertel @OskarStark @GromNaN @carsonbot