Closed phillbaker closed 7 years ago
Hey @phillbaker!
Like you said, I'd prefer if such a feature is completely optional.
We could do this by detecting if the required dependencies for some particular storage backend are installed (instead adding these dependencies in setup.py
for everybody), kinda like we usually do when dealing with deprecated modules:
# In the module that implements the Redis-based persistence factory, or whatever
try:
import dependency
except ImportError:
# Module does not exist, so this feature is not available
# Raise an error explaining this could only be used if the required dependencies exist
As for the implementation, all I can say is to keep it as simple as possible. It sure looks the implementation in breaker_box is simple enough, so that's fine by me.
One more thing is that you need to be careful so that any problems communicating with the storage backend do not interfere with the circuit breaker (and thus not cause application errors) if the storage is offline.
Thanks! I'll see where we end up and if it makes sense to PR back to this repo. Thanks for the work on this!
Would a PR to add external state be accepted?
We're looking at using pybreaker to manage the healthiness of an external API and we'd like to aggregate exceptions/failures across multiple processes/machines. For a first pass we'll probably throw data in redis, but I'd understand not wanting to add that as a dependency to this library. We were thinking of something similar to the pattern described here: https://github.com/sittercity/breaker_box#persistence-factories, which abstracts the storage a bit. What do you think would be a good approach that fits?