Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add concurrency models #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

webkonstantin
Copy link

Here is my take on concurrency based on async.queue and bluedird's cancellable promises. It represents shell commands as promises, which when cancelled would kill the underlying process. It also puts commands into the queue to allow fine-grained control over concurrency models.

I've also utilized exec-sh which abstracts away cross platform command execution and stdio stream forwarding.

This PR adds --concurrency option with three possible concurrency model choices:

  • kill (default): kills unfinished process before starting a new one. Example use case: assets compilation. For example if *.sass file changes you want to kill current build process and start a new one.
  • queue: waits until previously started process is finished before starting a new one. Example use case: continuous rsync on file changes. If file changes during syncing you probably want to let rsync finish and run it once more after it's done.
  • parallel: executes subsequent commands in parallel. I believe this option is how chokidar-cli works currently.

Note that this PR needs some clean up before it is ready to merge. I'll clean it up and work on tests if you like this approach.

if (process) {
console.log(('Killing run #' + task.number).yellow);

// XXX: should we send SIGKILL or account for sub-processes somehow
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think killing the subprocesses should be a requirement. in my use cases, the subprocess ends up being /bin/sh, which launches npm, which launches another /bin/sh. sending SIGINT to the shell won't help kill the rest of the subprocesses. the 2 ways i found to work around this were to launch the subprocess in a process group, or to traverse the output of ps to kill each process. the first solution has issues on OS X. the second is abstracted nicely in npm module ps-tree. see #29 for more details.

@aoberoi
Copy link

aoberoi commented Mar 31, 2016

@sw-double this looks really cool! i think it has a superset of the goals of the PR i made a few days earlier (#29). i'd be happy to see this land instead of mine if i saw some tests that verified the functionality works as intended. did you try it out in a project?

@corysimmons
Copy link

I would love to see this get in. It'd make chokidar so useful for "live reloading" Express/Koa servers.

@sw-double could you add a few tests so this gets merged? :bowtie:

@jonatandorozco
Copy link

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants