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

Make it able to track progress #7

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

skogsmaskin
Copy link

@skogsmaskin skogsmaskin commented May 2, 2016

Hi rse!

I've made some changes to the code in order to facilitate progress tracking and better logging as the job progresses.

I've changed the .execute code to perform spawn instead of execFile, which allows us to handle stdout and stderr as they arrive. These can now be caputred line by line by chaning the Prince call with:

  .on('stderr', function(line, prince) { ... })
  .on('stdout', function(line, prince) { ... })

Registering multiple event handlers on the same events is possible too.

Price supports the option --structured-log=progress which can be used to track progress of the job.
This can now be achieved like this:

Prince()
  .inputs(...)
  .output(...)
  .option('structured-log', 'progress')
  .on('stderr', trackProgress)
  .execute()

The trackProgress function may look something like this:

function trackProgress(line, prince) {
  if (line.substring(0, 4) === 'prg|') {
    const value = parseInt(line.split('|')[1], 0)
    console.log(`${value}% done`)

    // This is also possible (for example value only)
    if (value > 49) {
      prince.off('stderr', this);
      console.log('I don't want to track it no more than 50%!')
    }

  }
}

Not sure if this is an ideal implementation though. First of, Prince doesn't seem to output anything to stdout, but I implemented a stdout-handler as well, as I'm not sure if other versions of prince may output something to stdout.

Also, it is maybe a little strange with .on handlers when things like error is handled as a promise rejection (kind of a mix of two different patterns). In this regard the chaining function name could be called .onOutput instead. However I find the .on pattern to be familiar and flexible for possible future expansions.

What do you think?

arthurattwell added a commit to electricbookworks/node-prince that referenced this pull request Jan 14, 2022
arthurattwell added a commit to electricbookworks/node-prince that referenced this pull request Jan 14, 2022
Implement logging, copied from bengler/node-prince rse#7
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.

2 participants