-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add Support for Redis as a Persistence Database in Flower #1411
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: master
Are you sure you want to change the base?
Add Support for Redis as a Persistence Database in Flower #1411
Conversation
|
@NitinSingh8 Does this give an option to store the flower data in redis instead of flower db file? . At present the storing the data in gdbm is creating issues as reported here DB Flower file corrupted |
Yes, it stores the flower data in redis instead of storing flower db file. |
| help="flower database file") | ||
| define("persistent", type=bool, default=False, | ||
| help="enable persistent mode") | ||
| define("redis_host", type=str, default=None, |
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.
is there a reason we can't support a redis URL containing all of these params?
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.
Pull Request Overview
This PR adds Redis support as a persistence backend for Flower, allowing users to store and retrieve state data using Redis instead of file-based storage. This provides a more scalable and distributed solution for state persistence.
- Added Redis configuration options (host, port, database, SSL, key) to command-line arguments
- Modified Events class to support both Redis and file-based persistence backends
- Updated application initialization to pass Redis parameters to Events class
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| flower/options.py | Adds new command-line options for Redis configuration |
| flower/events.py | Implements Redis persistence logic alongside existing file-based storage |
| flower/app.py | Passes Redis configuration parameters to Events constructor |
| README.rst | Documents Redis usage with example command |
| self.redis_key = redis_key | ||
|
|
||
| # Check if we are using Redis as the database for persistence | ||
| self.redis_as_db = self.redis_host and self.redis_port and self.redis_db |
Copilot
AI
Aug 12, 2025
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.
The condition for enabling Redis persistence is incorrect. When redis_db is 0 (a valid Redis database), the expression evaluates to False, preventing Redis from being used. Change to: self.redis_as_db = self.redis_host and self.redis_port is not None and self.redis_db is not None
| self.redis_as_db = self.redis_host and self.redis_port and self.redis_db | |
| self.redis_as_db = self.redis_host and self.redis_port is not None and self.redis_db is not None |
| state.close() | ||
| if self.redis_as_db: | ||
| logger.debug("Loading state from Redis...") | ||
| redis_client = redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl) |
Copilot
AI
Aug 12, 2025
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.
Redis client creation is duplicated in both load and save operations. Consider creating a helper method to avoid code duplication and ensure consistent connection parameters.
| redis_client = redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl) | ||
| state_data = redis_client.get(self.redis_key) | ||
| redis_client.close() | ||
| if state_data: | ||
| self.state = pickle.loads(state_data) | ||
| logger.debug(f"State loaded from Redis: {self.state}") |
Copilot
AI
Aug 12, 2025
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.
Redis connections should be properly managed using context managers or try/finally blocks to ensure cleanup even if exceptions occur. Consider using with redis.Redis(...) as redis_client: or wrap in try/finally.
| redis_client = redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl) | |
| state_data = redis_client.get(self.redis_key) | |
| redis_client.close() | |
| if state_data: | |
| self.state = pickle.loads(state_data) | |
| logger.debug(f"State loaded from Redis: {self.state}") | |
| with redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl) as redis_client: | |
| state_data = redis_client.get(self.redis_key) | |
| if state_data: | |
| self.state = pickle.loads(state_data) | |
| logger.debug(f"State loaded from Redis: {self.state}") |
| redis_client.close() | ||
| if state_data: | ||
| self.state = pickle.loads(state_data) | ||
| logger.debug(f"State loaded from Redis: {self.state}") |
Copilot
AI
Aug 12, 2025
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.
Logging the entire state object may expose sensitive information. Consider logging only a summary or removing this debug statement to prevent potential data leakage.
| logger.debug(f"State loaded from Redis: {self.state}") | |
| logger.debug(f"State loaded from Redis: type={type(self.state).__name__}, event_count={len(getattr(self.state, 'events', []))}") |
| state_data = pickle.dumps(self.state) | ||
| redis_client.set(self.redis_key, state_data) | ||
| redis_client.close() |
Copilot
AI
Aug 12, 2025
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.
Same as the load operation, Redis connections should be properly managed using context managers or try/finally blocks to ensure cleanup even if exceptions occur.
| state_data = pickle.dumps(self.state) | |
| redis_client.set(self.redis_key, state_data) | |
| redis_client.close() | |
| try: | |
| state_data = pickle.dumps(self.state) | |
| redis_client.set(self.redis_key, state_data) | |
| finally: | |
| redis_client.close() |
|
|
||
| In this example, Flower is using the `tasks.app` defined in the `examples/tasks.py <https://github.com/mher/flower/blob/master/examples/tasks.py>`_ file | ||
|
|
||
| ## Use Redis as a database for Persistence |
Copilot
AI
Aug 12, 2025
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.
The heading uses Markdown syntax (##) in a reStructuredText file. Use reStructuredText syntax instead: a title followed by equal signs or dashes on the next line.
| ## Use Redis as a database for Persistence | |
| Use Redis as a database for Persistence | |
| --------------------------------------- |
Summary
This pull request introduces support for using Redis as a persistence database in Flower. This enhancement allows users to store and retrieve the state data using Redis, providing a more scalable and distributed solution compared to file-based storage.
Key Changes
flower/events.pyto support Redis as a backend for state persistence.redis_host,redis_port,redis_db,redis_key, andredis_ssl.redis.StrictRedis.shelve.Usage
To use Redis as a persistence database, run Flower with the following command-line arguments:
bash
celery -A tasks.app flower --broker=amqp://guest:guest@localhost:5672// --redis_host=localhost --redis_port=6379 --redis_db=0 --redis_key=flower --redis_ssl=TrueBenefits
Testing
Notes
flower, but it can be customized as needed.