Skip to content

[DCP - Ingestion] Allow Ingestion Helper to start clearing the redis cache#2035

Open
gmechali wants to merge 1 commit into
datacommonsorg:masterfrom
gmechali:redis
Open

[DCP - Ingestion] Allow Ingestion Helper to start clearing the redis cache#2035
gmechali wants to merge 1 commit into
datacommonsorg:masterfrom
gmechali:redis

Conversation

@gmechali
Copy link
Copy Markdown
Contributor

This needs to be triggered by the workflow at the end of every workflow

return jsonify({'status': 'SUCCESS', 'message': 'Cache cleared'}), 200
except Exception as e:
logging.error(f"Failed to flush Redis cache: {e}")
return jsonify({'status': 'ERROR', 'message': str(e)}), 500
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new clear_redis_cache action to the ingestion helper, enabling asynchronous flushing of the Redis cache using environment-configured host and port settings. The redis library has been added to the project dependencies. Review feedback recommends using a context manager for the Redis connection and specifying a connection timeout to ensure better resource management and prevent potential hangs.

Comment on lines +298 to +300
import redis
r = redis.Redis(host=redis_host, port=int(redis_port))
r.flushall(asynchronous=True)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

It is recommended to use a context manager for the Redis connection to ensure that the connection pool is properly managed and resources are released. Additionally, specifying a socket_connect_timeout is a best practice to prevent the function from hanging indefinitely if the Redis server is unreachable or the network is slow.

Suggested change
import redis
r = redis.Redis(host=redis_host, port=int(redis_port))
r.flushall(asynchronous=True)
import redis
with redis.Redis(host=redis_host, port=int(redis_port), socket_connect_timeout=5) as r:
r.flushall(asynchronous=True)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants