Add Support for Redis as a Persistence Database in Flower#1411
Add Support for Redis as a Persistence Database in Flower#1411NitinSingh8 wants to merge 5 commits intomher:masterfrom
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.
is there a reason we can't support a redis URL containing all of these params?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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}") |
There was a problem hiding this comment.
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}") |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.