This repository was archived by the owner on Nov 21, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 3
cleanup+redis cache config error #1
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| import json | ||
| import logging | ||
| from aries_cloudagent.core.profile import Profile | ||
| from aries_cloudagent.cache.base import BaseCache | ||
| import ssl | ||
| from typing import Any, Sequence, Text, Union | ||
|
|
||
| import aioredis | ||
| import ssl | ||
| import json | ||
| from aries_cloudagent.cache.base import BaseCache | ||
| from aries_cloudagent.core.profile import Profile | ||
| from aries_cloudagent.core.error import BaseError | ||
|
|
||
| LOGGER = logging.getLogger(__name__) | ||
|
|
||
|
|
@@ -29,32 +30,15 @@ def __init__(self, root_profile: Profile) -> None: | |
| plugin_config = root_profile.settings["plugin_config"] or {} | ||
| config = plugin_config[self.config_key] | ||
| self.connection = config["connection"] | ||
| except KeyError as error: | ||
| raise OutboundQueueConfigurationError( | ||
| "Configuration missing for redis queue" | ||
| ) from error | ||
| try: | ||
| plugin_config = root_profile.settings["plugin_config"] or {} | ||
| config = plugin_config[self.config_key] | ||
| credentials = config["credentials"] | ||
| username = credentials["username"] | ||
| password = credentials["password"] | ||
| except KeyError as error: | ||
| pass | ||
| # raise OutboundQueueConfigurationError( | ||
| # "Configuration missing for redis queue" | ||
| # ) from error | ||
| try: | ||
| plugin_config = root_profile.settings["plugin_config"] or {} | ||
| config = plugin_config[self.config_key] | ||
| lssl = config["ssl"] | ||
| ca_cert = lssl["cacerts"] | ||
| except KeyError as error: | ||
| pass | ||
| # raise OutboundQueueConfigurationError( | ||
| # "Configuration missing for redis queue" | ||
| # ) from error | ||
|
|
||
| raise RedisCacheConfigurationError( | ||
| "Configuration missing for redis queue" | ||
| ) from error | ||
| self.prefix = config.get("prefix", "acapy") | ||
| self.pool = aioredis.ConnectionPool.from_url( | ||
| self.connection, max_connections=10, | ||
|
|
@@ -78,7 +62,7 @@ async def get(self, key: Text): | |
| if response is not None: | ||
| response = json.loads(response) | ||
| return response | ||
| pass | ||
|
|
||
| async def set(self, keys: Union[Text, Sequence[Text]], value: Any, ttl: int = None): | ||
| """ | ||
| Add an item to the cache with an optional ttl. | ||
|
|
@@ -93,10 +77,12 @@ async def set(self, keys: Union[Text, Sequence[Text]], value: Any, ttl: int = No | |
| LOGGER.debug("set:%s value:%s ttl:%d", keys, value, ttl) | ||
| # self._remove_expired_cache_items() | ||
| # expires_ts = time.perf_counter() + ttl if ttl else None | ||
| for key in [keys] if isinstance(keys, Text) else keys: | ||
| # self._cache[key] = {"expires": expires_ts, "value": value} | ||
| await self.redis.set(f"ACA-Py:{key}", json.dumps(value), ex=ttl) | ||
|
|
||
| try: | ||
| for key in [keys] if isinstance(keys, Text) else keys: | ||
| # self._cache[key] = {"expires": expires_ts, "value": value} | ||
| await self.redis.set(f"ACA-Py:{key}", json.dumps(value), ex=ttl) | ||
| except aioredis.RedisError as error: | ||
| raise RedisCacheSetKeyValueError("Unexpected redis client exception") from error | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea! Should there be one for the GET command as well?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was not sure if not getting something would be an error. maybe a connection to Redis died and we could wrap that error with a get. Not sure. I think I am in favor of another error type for 'get'. |
||
|
|
||
|
|
||
| async def clear(self, key: Text): | ||
|
|
@@ -107,9 +93,23 @@ async def clear(self, key: Text): | |
| key: the key to remove | ||
|
|
||
| """ | ||
| #TODO: clear redis cache given a key | ||
| # TODO: clear redis cache given a key | ||
| pass | ||
|
|
||
| async def flush(self): | ||
| """Remove all items from the cache.""" | ||
| pass | ||
| pass | ||
|
|
||
| class RedisCacheConfigurationError(BaseError): | ||
| """An error with the redis cache configuration.""" | ||
|
|
||
| def __init__(self, message): | ||
| """Initialize the exception instance.""" | ||
| super().__init__(message) | ||
|
|
||
| class RedisCacheSetKeyValueError(BaseError): | ||
| """An error with the redis cache set key.""" | ||
|
|
||
| def __init__(self, message): | ||
| """Initialize the exception instance.""" | ||
| super().__init__(message) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Part of the problem with combining all of these is that it makes username/passwords/SSL required configuration options. Most people when using redis just end up using the default user with no password and they don't bother setting up SSL. This would have been the case had I done my development with containers, but using my own redis server presented the complications of more strict security settings. The only config option that was required here was
config["connection"]. All other options{credentials:{username,password}, ssl:{cacerts}}are (and should be) optional.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.
aww! I understand the catch and pass now. there is an unused ssl.context hanging around, I assumed there was more at hand than I saw. I agree with you on the optional configs.