hook: React to commands failures

[jhermann]

I noticed that hook.py uses subprocess.Popen(command_pieces).wait() (BTW, why not subprocess.call), and does not check the return code (you ignore the wait result). It also might be you thought this code does what check_call does (namely, throw on non-OK exit). Right now, your exception handler is only triggered when the command cannot be found.

An improvement would be to log a warning on non-OK exit code, and allow failing commands to exit the beets process. Example:

hook:
  hooks:
    - event: import
      command: chmod -R go+rX "{lib.directory}"
      on_error: abort

Choices for on_error would be abort, ignore (the default), log, skip_album, and skip_item.

skip_* would depend on the event, e.g. skip an album import if the command fails.

[sampsyo]

Sounds good!

I’m not aware of a reason why that code doesn’t use the higher-level wrappers in subprocess. That would be nice to fix too.

[jackwilsdon]

I wouldn't mind looking into this, however I'm not quite sure how the skip functionality would work. Do you have any specific events in mind for the skip operations @jhermann? The only tasks I can see which it would really work for are import_task_created and maybe a few of the other import_task_* events.

Similarly, how would abort work? Would it just throw an exception to force beets to exit? It seems like most events don't provide any cancellation functionality, so that looks like it might be the only way to do it.

Would ignore stop logging errors? If so then we should probably default to log which is what we do right now.

[anarcat]

Do you have any specific events in mind for the skip operations @jhermann? The only tasks I can see which it would really work for are import_task_created and maybe a few of the other import_task_* events.

old issue, but yeah, this would be relevant to avoid messing around with files that are handled by another manager, say a torrent client (#2617).