cleanup+redis cache config error #1
Conversation
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Signed-off-by: Adam Burdett <burdettadam@gmail.com>
|
|
||
| raise RedisCacheConfigurationError( | ||
| "Configuration missing for redis queue" | ||
| ) from error |
There was a problem hiding this comment.
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.
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.
| # 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 |
There was a problem hiding this comment.
Great idea! Should there be one for the GET command as well?
There was a problem hiding this comment.
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'.
cleaned up some code and added an error for failed configurations and cache key-value setting.