-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 hook plugin (fixes #1561) #1603
Changes from 11 commits
88ece41
2d0c217
1378351
8fea1e6
0af47bf
6a56677
553dd1f
276c5da
8b7af7f
46e5f9d
3d058f4
ae2ff61
0dff24e
417a724
55bd513
dd949a9
8b4f349
686e069
3e35660
af5ce6e
070469e
028c78a
dea091e
ffa2fdd
85fd608
7c9440c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
# This file is part of beets. | ||
# Copyright 2015, Adrian Sampson. | ||
# | ||
# Permission is hereby granted, free of charge, to any person obtaining | ||
# a copy of this software and associated documentation files (the | ||
# "Software"), to deal in the Software without restriction, including | ||
# without limitation the rights to use, copy, modify, merge, publish, | ||
# distribute, sublicense, and/or sell copies of the Software, and to | ||
# permit persons to whom the Software is furnished to do so, subject to | ||
# the following conditions: | ||
# | ||
# The above copyright notice and this permission notice shall be | ||
# included in all copies or substantial portions of the Software. | ||
|
||
"""Allows custom commands to be run when an event is emitted by beets""" | ||
from __future__ import (division, absolute_import, print_function, | ||
unicode_literals) | ||
|
||
import subprocess | ||
import sys | ||
|
||
from beets.plugins import BeetsPlugin | ||
|
||
|
||
def create_hook_function(log, event, command, shell, substitute_args): | ||
|
||
# TODO: Find a better way of piping STDOUT/STDERR/STDIN between the process | ||
# and the user. | ||
# | ||
# The issue with our current method is that we can only pesudo-pipe | ||
# one (two if we count STDERR being piped to STDOUT) stream at a | ||
# time, meaning we can't have both output and input simultaneously. | ||
# This is due to how Popen.std(out/err) works, as | ||
# Popen.std(out/err).readline() waits until a newline has been output | ||
# to the stream before returning. | ||
|
||
# TODO: Find a better way of converting arguments to strings, as I | ||
# currently have a feeling that forcing everything to utf-8 might | ||
# end up causing a mess. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed; we'll need to encode the strings as bytes using the platform's preferred encoding. That's the same encoding we use for decoding arguments, from the |
||
|
||
def hook_function(**kwargs): | ||
hook_command = command | ||
|
||
for key in substitute_args: | ||
if key in kwargs: | ||
hook_command = hook_command.replace(substitute_args[key], | ||
unicode(kwargs[key], | ||
"utf-8")) | ||
|
||
log.debug('Running command {0} for event {1}', hook_command, event) | ||
|
||
process = subprocess.Popen(hook_command, | ||
stdout=subprocess.PIPE, | ||
stderr=subprocess.STDOUT, | ||
shell=shell) | ||
|
||
while process.poll() is None: | ||
sys.stdout.write(process.stdout.readline()) | ||
|
||
# Ensure there's nothing left in the stream | ||
sys.stdout.write(process.stdout.readline()) | ||
|
||
return hook_function | ||
|
||
|
||
class HookPlugin(BeetsPlugin): | ||
"""Allows custom commands to be run when an event is emitted by beets""" | ||
def __init__(self): | ||
super(HookPlugin, self).__init__() | ||
|
||
self.config.add({ | ||
'hooks': [], | ||
'substitute_event': '%EVENT%', | ||
'shell': True | ||
}) | ||
|
||
hooks = self.config['hooks'].get(list) | ||
global_substitute_event = self.config['substitute_event'].get() | ||
global_shell = self.config['shell'].get(bool) | ||
|
||
for hook_index in range(len(hooks)): | ||
hook = self.config['hooks'][hook_index] | ||
|
||
hook_event = hook['event'].get() | ||
hook_command = hook['command'].get() | ||
|
||
if 'substitute_event' in hook: | ||
original = hook['substitute_event'].get() | ||
else: | ||
original = global_substitute_event | ||
|
||
if 'shell' in hook: | ||
shell = hook['shell'].get(bool) | ||
else: | ||
shell = global_shell | ||
|
||
if 'substitute_args' in hook: | ||
substitute_args = hook['substitute_args'].get(dict) | ||
else: | ||
substitute_args = {} | ||
|
||
hook_command = hook_command.replace(original, hook_event) | ||
hook_function = create_hook_function(self._log, hook_event, | ||
hook_command, shell, | ||
substitute_args) | ||
|
||
self.register_listener(hook_event, hook_function) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
Hook Plugin | ||
=============== | ||
|
||
Internally, beets sends events to plugins when an action finishes. These can | ||
range from importing a song (``import``) to beets exiting (``cli_exit``), and | ||
provide a very flexible way to perform actions based on the events. This plugin | ||
allows you to run commands when an event is emitted by beets, such as syncing | ||
your library with another drive when the library is updated. | ||
|
||
Hooks are currently run in the order defined in the configuration, however this | ||
is dependent on beets itself (may change in the future) and cannot be controlled | ||
by this plugin. | ||
|
||
.. _hook-configuration: | ||
|
||
Configuration | ||
------------- | ||
|
||
To configure the plugin, make a ``hook:`` section in your configuration | ||
file. The available options are: | ||
|
||
- **hooks**: A list of events and the commands to run (see :ref:`individual-hook-configuration`) | ||
Default: Empty. | ||
- **substitute_event**: The string to replace in each command with the name of | ||
the event executing it. This can be used to allow one script to act | ||
differently depending on the event it was called by. Can be individually | ||
overridden (see :ref:`individual-hook-configuration`). | ||
Default: ``%EVENT%`` | ||
- **shell**: Run each command in a shell. Can be individually overridden (see | ||
:ref:`individual-hook-configuration`). | ||
Default: ``yes`` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I'm wrong, but we nearly always want There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After doing some research, I'm not entirely sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, not using a shell by default would certainly be more secure! But if we bypass it entirely by default, then a configuration like this:
won't work (as things currently stand). The plugin will try to find an executable named We could, however, use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So after implementing
What do you think of catching the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've gone ahead and added the exception modification code in 8b4f349. It's not exactly pretty but it works. The error now looks like this:
I admit it could still do with some work though, as it's not inherently obvious that it's from the hook plugin. |
||
|
||
.. _individual-hook-configuration: | ||
|
||
Individual Hook Configuration | ||
----------------------------- | ||
|
||
Each element of the ``hooks`` configuration option can be configured separately. | ||
The available options are: | ||
|
||
- **event** (required): The name of the event that should cause this hook to execute. See | ||
:ref:`Plugin Events <plugin_events>` for a list of possible values. | ||
- **command** (required): The command to run when this hook executes. | ||
- **substitute_event**: Hook-level override for ``substitute_event`` option in | ||
:ref:`hook-configuration`. | ||
Default: Value of top level ``substitute_event`` option (see :ref:`hook-configuration`) | ||
- **shell**: Hook-level override for ``shell`` option in :ref:`hook-configuration`. | ||
Default: Value of top level ``shell`` option (see :ref:`hook-configuration`). | ||
- **substitute_args**: A key/value set where the key is the name of the an | ||
argument passed to the event (see :ref:`Plugin Events <plugin_events>` for | ||
a list of arguments for each event) and the value is the string to replace | ||
in the command with the value of the argument. Note that any arguments that | ||
are not strings will be converted to strings (e.g. Python objects). | ||
Default: Empty. | ||
|
||
Example Configuration | ||
--------------------- | ||
|
||
.. code-block:: yaml | ||
|
||
hook: | ||
hooks: | ||
# Output on exit: | ||
# beets just exited! | ||
# have a nice day! | ||
- event: cli_exit | ||
command: echo "beets just exited!" | ||
- event: cli_exit | ||
command: echo "have a nice day!" | ||
|
||
# Output on write | ||
# writing to "<file_name_here>" | ||
# Where <file_name_here> is the file being written to | ||
- event: write | ||
command: echo "writing to \"%FILE_NAME%\"" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A somewhat simpler strategy would be to just use ordinary template substitution for the commands. That is, this would just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hadn't heard of template substitution, however it does seem to be a much better way of implementing it. I'll try it out now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So after doing some more work, it looks like template substitution solves half of the problem. What I'd like to do is allow the user to access object attributes, e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm… perhaps we could figure out how to use Python's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added this in 8b4f349, you can now use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome! This looks great! |
||
substitute_args: | ||
path: %FILE_NAME% |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
# This file is part of beets. | ||
# Copyright 2015, Thomas Scholtes. | ||
# | ||
# Permission is hereby granted, free of charge, to any person obtaining | ||
# a copy of this software and associated documentation files (the | ||
# "Software"), to deal in the Software without restriction, including | ||
# without limitation the rights to use, copy, modify, merge, publish, | ||
# distribute, sublicense, and/or sell copies of the Software, and to | ||
# permit persons to whom the Software is furnished to do so, subject to | ||
# the following conditions: | ||
# | ||
# The above copyright notice and this permission notice shall be | ||
# included in all copies or substantial portions of the Software. | ||
|
||
from __future__ import (division, absolute_import, print_function, | ||
unicode_literals) | ||
|
||
import os.path | ||
import tempfile | ||
|
||
from test import _common | ||
from test._common import unittest | ||
from test.helper import TestHelper | ||
|
||
from beets import config | ||
from beets import plugins | ||
|
||
|
||
def get_temporary_path(): | ||
temporary_directory = tempfile._get_default_tempdir() | ||
temporary_name = next(tempfile._get_candidate_names()) | ||
|
||
return os.path.join(temporary_directory, temporary_name) | ||
|
||
|
||
# TODO: Find a good way to test shell option | ||
class HookTest(_common.TestCase, TestHelper): | ||
TEST_HOOK_COUNT = 5 | ||
|
||
def setUp(self): | ||
self.setup_beets() # Converter is threaded | ||
|
||
def tearDown(self): | ||
self.unload_plugins() | ||
self.teardown_beets() | ||
|
||
def _add_hook(self, event, command, substitute_event=None, shell=None, | ||
substitute_args=None): | ||
|
||
hook = { | ||
'event': event, | ||
'command': command | ||
} | ||
|
||
if substitute_event is not None: | ||
hook['substitute_event'] = substitute_event | ||
|
||
if shell is not None: | ||
hook['shell'] = shell | ||
|
||
if substitute_args is not None: | ||
hook['substitute_args'] = substitute_args | ||
|
||
hooks = config['hook']['hooks'].get(list) if 'hook' in config else [] | ||
hooks.append(hook) | ||
|
||
config['hook']['hooks'] = hooks | ||
|
||
def test_hook_no_arguments(self): | ||
temporary_paths = [ | ||
get_temporary_path() for i in range(self.TEST_HOOK_COUNT) | ||
] | ||
|
||
for index, path in enumerate(temporary_paths): | ||
self._add_hook('test_event_{0}'.format(index), | ||
'echo > "{0}"'.format(path)) | ||
|
||
self.load_plugins('hook') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This actually seems OK to me -- it might look a little weird, of course, that the configuration comes first, but it's logical that the config would need to be in place as early as possible. |
||
|
||
for index in range(len(temporary_paths)): | ||
plugins.send('test_event_{0}'.format(index)) | ||
|
||
for path in temporary_paths: | ||
self.assertTrue(os.path.isfile(path)) | ||
os.remove(path) | ||
|
||
def test_hook_event_substitution(self): | ||
|
||
temporary_directory = tempfile._get_default_tempdir() | ||
event_names = ['test_event_{0}'.format(i) for i in | ||
range(self.TEST_HOOK_COUNT)] | ||
|
||
for event in event_names: | ||
self._add_hook(event, | ||
'echo > "{0}"'.format( | ||
os.path.join(temporary_directory, '%EVENT%') | ||
)) | ||
|
||
self.load_plugins('hook') | ||
|
||
for event in event_names: | ||
plugins.send(event) | ||
|
||
for event in event_names: | ||
path = os.path.join(temporary_directory, event) | ||
|
||
self.assertTrue(os.path.isfile(path)) | ||
os.remove(path) | ||
|
||
def test_hook_argument_substitution(self): | ||
temporary_paths = [ | ||
get_temporary_path() for i in range(self.TEST_HOOK_COUNT) | ||
] | ||
|
||
for index, path in enumerate(temporary_paths): | ||
self._add_hook('test_event_{0}'.format(index), | ||
'echo > "%PATH%"'.format(path), | ||
substitute_args={'path': '%PATH%'}) | ||
|
||
self.load_plugins('hook') | ||
|
||
for index, path in enumerate(temporary_paths): | ||
plugins.send('test_event_{0}'.format(index), path=path) | ||
|
||
for path in temporary_paths: | ||
self.assertTrue(os.path.isfile(path)) | ||
os.remove(path) | ||
|
||
|
||
def suite(): | ||
return unittest.TestLoader().loadTestsFromName(__name__) | ||
|
||
if __name__ == b'__main__': | ||
unittest.main(defaultTest='suite') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it work to just leave off the
stdout
andstderr
arguments toPopen
? They'll use the default behavior, which is to share the stdin/stdout/stderr of the parent process.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like removing the
stdout
andstderr
arguments does work, I can't believe I didn't try that earlier.Still can't read fromWell it turns out we can! Waiting for the process to exit usingstdin
though, which is a pain but I don't think is that big of an issue (interactive hooks would be better as their own plugins anyway).process.wait()
allowsstdin
to work too!