search

syrusakbary / promise

Ultra-performant Promise implementation in Python
MIT License
362 stars 76 forks source link

Fix Promise.all hanging in certain cases #14

Closed Globegitter closed 8 years ago

Globegitter commented 8 years ago

This is an issue very difficult to reproduce and it just happened in our live system. We are using GRPC, which returns us their own implementation of a Future (called _Rendezvous) and for some reason in some rare case, using Promise.all (as well as Promise.for_dict) the entire graphql request would hang. I have managed to boil it down to this line: https://github.com/syrusakbary/promise/blob/master/promise/promise.py#L410, specifically to the statement if p in promises. We have not been able to figure out why exactly it hangs on this comparison statement, but it might have something to do with the fact that we are using grpc futures under the hood (which have been promisified).

It was easy enough to rewrite this function to not have any comparison at all. We are using this patched version now in our live system so would be great to get it in (or some other way that does not need the p in promises comparison).

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.04%) to 94.815% when pulling b7cfd22264606c251c028ca8d16c3469691b91b9 on Globegitter:patch-4 into c743a221ed4856a45232eb44481e8d1bc1b8aaed on syrusakbary:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.04%) to 94.815% when pulling b7cfd22264606c251c028ca8d16c3469691b91b9 on Globegitter:patch-4 into c743a221ed4856a45232eb44481e8d1bc1b8aaed on syrusakbary:master.

Globegitter commented 8 years ago

Thanks for the quick merge :)