Async issues

[stj]

Just stumbled on your project and looked over your code to see if it would fit my use cases.
Two things immediately caught my eye and I like to raise them to the maintainer(s).

• threading.RLock is used in https://github.com/arlyon/aiobreaker/blob/master/aiobreaker/circuitbreaker.py#L32 which will not work as expected in asyncio.
  asyncio operates single-threaded and every async context will simple use the same lock. asyncio has its own Lock class.

• The redis storage is completely blocking code. Consider using aredis.

[arlyon]

Hey thanks for the interest. I didn't know that about RLock and will have a look at it soon. I haven't had need for redis (although that is changing soon) and haven't given it a critical look. I'd be happy to accept a PR to support it if youre desperate or otherwise I'll probably have some time to work on it soon.

[stj]

Adding aredis support will change the signature of every method to become a coro. Will be hard to do that in a backwards incompatible way. Before working on a PR I feel this should be talked through and agreed on.

[arlyon]

Yes you are right I think this library needs a bit of love. I'm going to rework the locking used in the original fork and look at the changes aioredis would require. I haven't even converted the original tests over yet so that would need doing as well.

[arlyon]

Alright so after a little bit of work (docs, CI, some fixes) we're at a point where adding this is reasonable. Looking at it again, using the lock makes no sense. It is rare that anyone is going to be using threads with asyncio anyways, so I ripped it out.

I've been trying to contain "async creep" as much as possible (which is why call and call_async a seperate) however like you said with aredis I think we have no choice but to go all in.

Ideally calling a synchronous function over the breaker shouldn't need to be awaited so with the need for async backends and async event listeners (that I'm about to implement) maybe some sort of "feature detection" would be useful. With the risk of over-complicating everything:

Storage Backend	Listeners	Supported Functions
Async	Sync	call_async
Sync	Async	call_async
Sync	Sync	call, call_async

That way completely synchronous code can still use the library synchronously if it doesn't require any async features.

I can do the needed work to allow for this, and then you can build the aredis module on top.

[stj]

Nice!
I have not read up on how the lock was used. Right now I feel that redis lock is useful for distributed services but I would not worry about that now.

Happy to look into the async redis support once you finished your refactor.