https://pantsbuild.org/ logo
#pex
Title
# pex
e

enough-analyst-54434

12/05/2019, 10:21 PM
If there is anyone willing to take a look at https://github.com/pantsbuild/pex/pull/819 I'd be grateful for more active reviewer eyeballs. Namely the producer/consumer concurrency limiter implemented here: https://github.com/pantsbuild/pex/blob/0fef27e144894aac91d4306bd4b51a929be07ea7/pex/jobs.py#L184-L243 If that bit of code is broken, you'd get inifinte hang and without that code you get ... nearly infinite hangs while your machine attempts to execute hundreds of concurrent wheel installs.
h

hundreds-father-404

12/05/2019, 10:24 PM
cc @aloof-angle-91616 if you have a moment today. I finished reviewing the whole PR just now and felt the least confident in my understanding with the lines John flags
a

aloof-angle-91616

12/05/2019, 10:24 PM
i have a large amount of moments today to devote to this
❤️ 1
was fighting random fires earlier
👨‍🚒 1
e

enough-analyst-54434

12/05/2019, 10:25 PM
Thanks guys.
a

aloof-angle-91616

12/05/2019, 10:28 PM
switching over to this now
thanks
reviewed, that code is significantly less confusing than the code in pants to download from the remote artifact cache (which i'm improving in a separate pants PR) so i feel great about approving it
💯 2
🎉 2
thanks a lot for iterating on this idea so speedily and providing incredibly good PR descriptions!
e

enough-analyst-54434

12/06/2019, 2:50 AM
I won't say this wasn't a detour, but I'm happy for where we're headed now.
Thanks best accepted in speedyish review of https://github.com/pantsbuild/pex/pull/821 if possible. If not that'll go out in pex 2.0.4 instead of 2.0.3: https://github.com/pantsbuild/pex/issues/814
a

aloof-angle-91616

12/06/2019, 2:51 AM
yes!
reviewing now
e

enough-analyst-54434

12/06/2019, 2:54 AM
Thank you!