search

cloudtools / stacker

An AWS CloudFormation Stack orchestrator/manager.
http://stacker.readthedocs.io/en/stable/
BSD 2-Clause "Simplified" License
711 stars 167 forks source link

ThreadedWalker constructor accepts an int, not a Semaphore #721

Closed acmcelwee closed 5 years ago

acmcelwee commented 5 years ago

This is an alternative to #720. IMO, it's a bit cleaner to have the threaded walker manage creation/ownership of the semaphore, based on the desired concurrency.

russellballestrini commented 5 years ago

I think I prefer this one. I'll let @ejholmes have final say.

ejholmes commented 5 years ago

I merged in https://github.com/cloudtools/stacker/pull/720. I definitely like the simplicity of this one, as it hides some of the details of how concurrency is implemented, but I generally don't like when constructors do too much, since you lose some flexibility (e.g. if you want to pass in a new semaphore implementation, you can't)