From 8dddc6d566c891346ec69780020bddf43b4e364a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 2 Apr 2013 12:14:09 +0200 Subject: [PATCH 01/29] http: Fix docs typo --- mopidy/frontends/http/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index f7f9f659..5e9629a7 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -427,7 +427,7 @@ Example to get started with .. code-block:: js - var consoleError = console.error.bind(error); + var consoleError = console.error.bind(console); var trackDesc = function (track) { return track.name + " by " + track.artists[0].name + @@ -457,7 +457,7 @@ Example to get started with .. code-block:: js - var consoleError = console.error.bind(error); + var consoleError = console.error.bind(console); var getFirst = function (list) { return list[0]; From 6f39bde566fa32190f724754993494d8e38f203a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 16:18:15 +0200 Subject: [PATCH 02/29] config: Start passing dummy config explicitly to audio/backends/frontends. --- mopidy/__main__.py | 30 ++++++++++--------- mopidy/audio/actor.py | 2 +- mopidy/backends/dummy.py | 6 +++- mopidy/backends/local/actor.py | 2 +- mopidy/backends/spotify/actor.py | 2 +- mopidy/backends/stream/actor.py | 2 +- mopidy/frontends/http/actor.py | 2 +- mopidy/frontends/lastfm/actor.py | 2 +- mopidy/frontends/mpd/actor.py | 2 +- mopidy/frontends/mpris/actor.py | 2 +- mopidy/utils/log.py | 2 +- tests/audio/actor_test.py | 6 ++-- tests/backends/base/events.py | 5 +++- tests/backends/base/library.py | 4 ++- tests/backends/base/playback.py | 4 ++- tests/backends/base/playlists.py | 5 +++- tests/backends/base/tracklist.py | 4 ++- tests/backends/local/events_test.py | 1 + tests/backends/local/library_test.py | 2 +- tests/backends/local/playback_test.py | 1 + tests/backends/local/playlists_test.py | 3 +- tests/backends/local/tracklist_test.py | 1 + tests/core/events_test.py | 2 +- tests/frontends/http/events_test.py | 2 +- tests/frontends/mpd/dispatcher_test.py | 2 +- tests/frontends/mpd/protocol/__init__.py | 2 +- tests/frontends/mpd/status_test.py | 2 +- tests/frontends/mpris/events_test.py | 2 +- .../frontends/mpris/player_interface_test.py | 2 +- .../mpris/playlists_interface_test.py | 2 +- tests/frontends/mpris/root_interface_test.py | 2 +- tests/utils/jsonrpc_test.py | 2 +- 32 files changed, 66 insertions(+), 44 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 2aeff179..4f531000 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -53,17 +53,18 @@ def main(): loop = gobject.MainLoop() options = parse_options() + config = {} # TODO: replace dummy placeholder try: - log.setup_logging(options.verbosity_level, options.save_debug_log) + log.setup_logging(config, options.verbosity_level, options.save_debug_log) check_old_folders() extensions = load_extensions() load_config(options, extensions) setup_settings(options.interactive) - audio = setup_audio() - backends = setup_backends(extensions, audio) + audio = setup_audio(config) + backends = setup_backends(config, extensions, audio) core = setup_core(audio, backends) - setup_frontends(extensions, core) + setup_frontends(config, extensions, core) loop.run() except exceptions.SettingsError as ex: logger.error(ex.message) @@ -122,6 +123,7 @@ def parse_options(): def check_old_folders(): + # TODO: add old settings and pre extension storage locations? old_settings_folder = os.path.expanduser('~/.mopidy') if not os.path.isdir(old_settings_folder): @@ -138,8 +140,6 @@ def load_extensions(): for entry_point in pkg_resources.iter_entry_points('mopidy.ext'): logger.debug('Loading extension %s', entry_point.name) - # TODO Filter out disabled extensions - try: extension_class = entry_point.load() except pkg_resources.DistributionNotFound as ex: @@ -157,8 +157,6 @@ def load_extensions(): {'ep': entry_point.name, 'ext': extension.ext_name}) continue - # TODO Validate configuration - try: extension.validate_environment() except exceptions.ExtensionError as ex: @@ -166,6 +164,10 @@ def load_extensions(): 'Disabled extension %s: %s', entry_point.name, ex.message) continue + # TODO: due to order we do things in we can't know for sure if we are + # going to use it at this point, should we perhaps just log a single + # line with all extenions we found and then log an enabled line for + # each one after we check configs etc? logger.info( 'Loaded extension %s: %s %s', entry_point.name, extension.dist_name, extension.version) @@ -240,9 +242,9 @@ def setup_settings(interactive): sys.exit(1) -def setup_audio(): +def setup_audio(config): logger.info('Starting Mopidy audio') - return Audio.start().proxy() + return Audio.start(config=config).proxy() def stop_audio(): @@ -250,12 +252,12 @@ def stop_audio(): process.stop_actors_by_class(Audio) -def setup_backends(extensions, audio): +def setup_backends(config, extensions, audio): logger.info('Starting Mopidy backends') backends = [] for extension in extensions: for backend_class in extension.get_backend_classes(): - backend = backend_class.start(audio=audio).proxy() + backend = backend_class.start(config=config, audio=audio).proxy() backends.append(backend) return backends @@ -277,11 +279,11 @@ def stop_core(): process.stop_actors_by_class(Core) -def setup_frontends(extensions, core): +def setup_frontends(config, extensions, core): logger.info('Starting Mopidy frontends') for extension in extensions: for frontend_class in extension.get_frontend_classes(): - frontend_class.start(core=core) + frontend_class.start(config=config, core=core) def stop_frontends(extensions): diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 11d2741f..42dee084 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -38,7 +38,7 @@ class Audio(pykka.ThreadingActor): #: The GStreamer state mapped to :class:`mopidy.audio.PlaybackState` state = PlaybackState.STOPPED - def __init__(self): + def __init__(self, config): super(Audio, self).__init__() self._playbin = None diff --git a/mopidy/backends/dummy.py b/mopidy/backends/dummy.py index dd021445..65477ea2 100644 --- a/mopidy/backends/dummy.py +++ b/mopidy/backends/dummy.py @@ -22,8 +22,12 @@ from mopidy.backends import base from mopidy.models import Playlist, SearchResult +def create_dummy_backend_proxy(config=None, audio=None): + return DummyBackend.start(config=config, audio=audio).proxy() + + class DummyBackend(pykka.ThreadingActor, base.Backend): - def __init__(self, audio): + def __init__(self, config, audio): super(DummyBackend, self).__init__() self.library = DummyLibraryProvider(backend=self) diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 75baeab2..abad75ca 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -13,7 +13,7 @@ logger = logging.getLogger('mopidy.backends.local') class LocalBackend(pykka.ThreadingActor, base.Backend): - def __init__(self, audio): + def __init__(self, config, audio): super(LocalBackend, self).__init__() self.library = LocalLibraryProvider(backend=self) diff --git a/mopidy/backends/spotify/actor.py b/mopidy/backends/spotify/actor.py index 5e90205b..67b4acdc 100644 --- a/mopidy/backends/spotify/actor.py +++ b/mopidy/backends/spotify/actor.py @@ -14,7 +14,7 @@ class SpotifyBackend(pykka.ThreadingActor, base.Backend): # Imports inside methods are to prevent loading of __init__.py to fail on # missing spotify dependencies. - def __init__(self, audio): + def __init__(self, config, audio): super(SpotifyBackend, self).__init__() from .library import SpotifyLibraryProvider diff --git a/mopidy/backends/stream/actor.py b/mopidy/backends/stream/actor.py index f80ac7a9..d6eb31d3 100644 --- a/mopidy/backends/stream/actor.py +++ b/mopidy/backends/stream/actor.py @@ -13,7 +13,7 @@ logger = logging.getLogger('mopidy.backends.stream') class StreamBackend(pykka.ThreadingActor, base.Backend): - def __init__(self, audio): + def __init__(self, config, audio): super(StreamBackend, self).__init__() self.library = StreamLibraryProvider(backend=self) diff --git a/mopidy/frontends/http/actor.py b/mopidy/frontends/http/actor.py index 8ad0f026..54085471 100644 --- a/mopidy/frontends/http/actor.py +++ b/mopidy/frontends/http/actor.py @@ -23,7 +23,7 @@ logger = logging.getLogger('mopidy.frontends.http') class HttpFrontend(pykka.ThreadingActor, CoreListener): - def __init__(self, core): + def __init__(self, config, core): super(HttpFrontend, self).__init__() self.core = core self._setup_server() diff --git a/mopidy/frontends/lastfm/actor.py b/mopidy/frontends/lastfm/actor.py index 60a909e0..1e157d4f 100644 --- a/mopidy/frontends/lastfm/actor.py +++ b/mopidy/frontends/lastfm/actor.py @@ -20,7 +20,7 @@ API_SECRET = '94d9a09c0cd5be955c4afaeaffcaefcd' class LastfmFrontend(pykka.ThreadingActor, CoreListener): - def __init__(self, core): + def __init__(self, config, core): super(LastfmFrontend, self).__init__() self.lastfm = None self.last_start_time = None diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 8907fe22..e288c24e 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -14,7 +14,7 @@ logger = logging.getLogger('mopidy.frontends.mpd') class MpdFrontend(pykka.ThreadingActor, CoreListener): - def __init__(self, core): + def __init__(self, config, core): super(MpdFrontend, self).__init__() hostname = network.format_hostname(settings.MPD_SERVER_HOSTNAME) port = settings.MPD_SERVER_PORT diff --git a/mopidy/frontends/mpris/actor.py b/mopidy/frontends/mpris/actor.py index 5e171826..11f87922 100644 --- a/mopidy/frontends/mpris/actor.py +++ b/mopidy/frontends/mpris/actor.py @@ -18,7 +18,7 @@ except ImportError as import_error: class MprisFrontend(pykka.ThreadingActor, CoreListener): - def __init__(self, core): + def __init__(self, config, core): super(MprisFrontend, self).__init__() self.core = core self.indicate_server = None diff --git a/mopidy/utils/log.py b/mopidy/utils/log.py index ae4ea0d9..d50f107f 100644 --- a/mopidy/utils/log.py +++ b/mopidy/utils/log.py @@ -7,7 +7,7 @@ from mopidy import settings from . import deps, versioning -def setup_logging(verbosity_level, save_debug_log): +def setup_logging(config, verbosity_level, save_debug_log): setup_root_logger() setup_console_logging(verbosity_level) if save_debug_log: diff --git a/tests/audio/actor_test.py b/tests/audio/actor_test.py index 73c8c165..35503472 100644 --- a/tests/audio/actor_test.py +++ b/tests/audio/actor_test.py @@ -17,7 +17,7 @@ class AudioTest(unittest.TestCase): settings.MIXER = 'fakemixer track_max_volume=65536' settings.OUTPUT = 'fakesink' self.song_uri = path_to_uri(path_to_data_dir('song1.wav')) - self.audio = audio.Audio.start().proxy() + self.audio = audio.Audio.start(None).proxy() def tearDown(self): pykka.ActorRegistry.stop_all() @@ -60,7 +60,7 @@ class AudioTest(unittest.TestCase): def test_set_volume_with_mixer_max_below_100(self): settings.MIXER = 'fakemixer track_max_volume=40' - self.audio = audio.Audio.start().proxy() + self.audio = audio.Audio.start(None).proxy() for value in range(0, 101): self.assertTrue(self.audio.set_volume(value).get()) @@ -81,7 +81,7 @@ class AudioTest(unittest.TestCase): class AudioStateTest(unittest.TestCase): def setUp(self): - self.audio = audio.Audio() + self.audio = audio.Audio(None) def test_state_starts_as_stopped(self): self.assertEqual(audio.PlaybackState.STOPPED, self.audio.state) diff --git a/tests/backends/base/events.py b/tests/backends/base/events.py index 1d31a721..a5d9fa7b 100644 --- a/tests/backends/base/events.py +++ b/tests/backends/base/events.py @@ -9,9 +9,12 @@ from mopidy.backends import listener @mock.patch.object(listener.BackendListener, 'send') class BackendEventsTest(object): + config = {} + def setUp(self): self.audio = audio.DummyAudio.start().proxy() - self.backend = self.backend_class.start(audio=self.audio).proxy() + self.backend = self.backend_class.start( + config=self.config, audio=self.audio).proxy() self.core = core.Core.start(backends=[self.backend]).proxy() def tearDown(self): diff --git a/tests/backends/base/library.py b/tests/backends/base/library.py index c75bec74..8390d2d6 100644 --- a/tests/backends/base/library.py +++ b/tests/backends/base/library.py @@ -23,9 +23,11 @@ class LibraryControllerTest(object): uri='file://' + path_to_data_dir('uri2'), name='track2', artists=artists[1:2], album=albums[1], date='2002', length=4000), Track()] + config = {} def setUp(self): - self.backend = self.backend_class.start(audio=None).proxy() + self.backend = self.backend_class.start( + config=self.config, audio=None).proxy() self.core = core.Core(backends=[self.backend]) self.library = self.core.library diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index e12d54a5..9ce73d31 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -18,10 +18,12 @@ from tests.backends.base import populate_tracklist class PlaybackControllerTest(object): tracks = [] + config = {} def setUp(self): self.audio = audio.DummyAudio.start().proxy() - self.backend = self.backend_class.start(audio=self.audio).proxy() + self.backend = self.backend_class.start( + config=self.config, audio=self.audio).proxy() self.core = core.Core(backends=[self.backend]) self.playback = self.core.playback self.tracklist = self.core.tracklist diff --git a/tests/backends/base/playlists.py b/tests/backends/base/playlists.py index 2184168f..00e32a6f 100644 --- a/tests/backends/base/playlists.py +++ b/tests/backends/base/playlists.py @@ -13,13 +13,16 @@ from tests import unittest, path_to_data_dir class PlaylistsControllerTest(object): + config = {} + def setUp(self): settings.LOCAL_PLAYLIST_PATH = tempfile.mkdtemp() settings.LOCAL_TAG_CACHE_FILE = path_to_data_dir('library_tag_cache') settings.LOCAL_MUSIC_PATH = path_to_data_dir('') self.audio = audio.DummyAudio.start().proxy() - self.backend = self.backend_class.start(audio=self.audio).proxy() + self.backend = self.backend_class.start( + config=self.config, audio=self.audio).proxy() self.core = core.Core(backends=[self.backend]) def tearDown(self): diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 39fb020d..5140d3aa 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -13,10 +13,12 @@ from tests.backends.base import populate_tracklist class TracklistControllerTest(object): tracks = [] + config = {} def setUp(self): self.audio = audio.DummyAudio.start().proxy() - self.backend = self.backend_class.start(audio=self.audio).proxy() + self.backend = self.backend_class.start( + config=self.config, audio=self.audio).proxy() self.core = core.Core(audio=self.audio, backends=[self.backend]) self.controller = self.core.tracklist self.playback = self.core.playback diff --git a/tests/backends/local/events_test.py b/tests/backends/local/events_test.py index b35fad1a..5ccf0886 100644 --- a/tests/backends/local/events_test.py +++ b/tests/backends/local/events_test.py @@ -7,6 +7,7 @@ from tests.backends.base import events class LocalBackendEventsTest(events.BackendEventsTest, unittest.TestCase): backend_class = actor.LocalBackend + # TODO: setup config def setUp(self): settings.LOCAL_TAG_CACHE_FILE = path_to_data_dir('empty_tag_cache') diff --git a/tests/backends/local/library_test.py b/tests/backends/local/library_test.py index 7bf8d565..ca90e40b 100644 --- a/tests/backends/local/library_test.py +++ b/tests/backends/local/library_test.py @@ -8,8 +8,8 @@ from tests.backends.base.library import LibraryControllerTest class LocalLibraryControllerTest(LibraryControllerTest, unittest.TestCase): - backend_class = actor.LocalBackend + # TODO: setup config def setUp(self): settings.LOCAL_TAG_CACHE_FILE = path_to_data_dir('library_tag_cache') diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 8d997d2e..e9b3954c 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -15,6 +15,7 @@ class LocalPlaybackControllerTest(PlaybackControllerTest, unittest.TestCase): backend_class = actor.LocalBackend tracks = [ Track(uri=generate_song(i), length=4464) for i in range(1, 4)] + # TODO: setup config def setUp(self): settings.LOCAL_TAG_CACHE_FILE = path_to_data_dir('empty_tag_cache') diff --git a/tests/backends/local/playlists_test.py b/tests/backends/local/playlists_test.py index f3794cee..3dbc3a2a 100644 --- a/tests/backends/local/playlists_test.py +++ b/tests/backends/local/playlists_test.py @@ -17,6 +17,7 @@ class LocalPlaylistsControllerTest( PlaylistsControllerTest, unittest.TestCase): backend_class = actor.LocalBackend + # TODO: setup config def setUp(self): settings.LOCAL_TAG_CACHE_FILE = path_to_data_dir('empty_tag_cache') @@ -96,7 +97,7 @@ class LocalPlaylistsControllerTest( playlist = playlist.copy(tracks=[track]) playlist = self.core.playlists.save(playlist) - backend = self.backend_class(audio=self.audio) + backend = self.backend_class(config=self.config, audio=self.audio) self.assert_(backend.playlists.playlists) self.assertEqual( diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 0c47a5db..24c400fa 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -13,6 +13,7 @@ class LocalTracklistControllerTest(TracklistControllerTest, unittest.TestCase): backend_class = actor.LocalBackend tracks = [ Track(uri=generate_song(i), length=4464) for i in range(1, 4)] + # TODO: setup config def setUp(self): settings.LOCAL_TAG_CACHE_FILE = path_to_data_dir('empty_tag_cache') diff --git a/tests/core/events_test.py b/tests/core/events_test.py index 11881db7..7f673b02 100644 --- a/tests/core/events_test.py +++ b/tests/core/events_test.py @@ -13,7 +13,7 @@ from tests import unittest @mock.patch.object(core.CoreListener, 'send') class BackendEventsTest(unittest.TestCase): def setUp(self): - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() def tearDown(self): diff --git a/tests/frontends/http/events_test.py b/tests/frontends/http/events_test.py index 77438fd4..7661ac6e 100644 --- a/tests/frontends/http/events_test.py +++ b/tests/frontends/http/events_test.py @@ -24,7 +24,7 @@ from tests import unittest @mock.patch('cherrypy.engine.publish') class HttpEventsTest(unittest.TestCase): def setUp(self): - self.http = actor.HttpFrontend(core=mock.Mock()) + self.http = actor.HttpFrontend(config=None, core=mock.Mock()) def test_track_playback_paused_is_broadcasted(self, publish): publish.reset_mock() diff --git a/tests/frontends/mpd/dispatcher_test.py b/tests/frontends/mpd/dispatcher_test.py index 3404db95..3c32cd32 100644 --- a/tests/frontends/mpd/dispatcher_test.py +++ b/tests/frontends/mpd/dispatcher_test.py @@ -13,7 +13,7 @@ from tests import unittest class MpdDispatcherTest(unittest.TestCase): def setUp(self): - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.dispatcher = MpdDispatcher() diff --git a/tests/frontends/mpd/protocol/__init__.py b/tests/frontends/mpd/protocol/__init__.py index 00594206..9d24c3fa 100644 --- a/tests/frontends/mpd/protocol/__init__.py +++ b/tests/frontends/mpd/protocol/__init__.py @@ -24,7 +24,7 @@ class MockConnection(mock.Mock): class BaseTestCase(unittest.TestCase): def setUp(self): - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.connection = MockConnection() diff --git a/tests/frontends/mpd/status_test.py b/tests/frontends/mpd/status_test.py index d508cbf0..8868eef7 100644 --- a/tests/frontends/mpd/status_test.py +++ b/tests/frontends/mpd/status_test.py @@ -22,7 +22,7 @@ STOPPED = PlaybackState.STOPPED class StatusHandlerTest(unittest.TestCase): def setUp(self): - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.dispatcher = dispatcher.MpdDispatcher(core=self.core) self.context = self.dispatcher.context diff --git a/tests/frontends/mpris/events_test.py b/tests/frontends/mpris/events_test.py index 78e40071..f79202c0 100644 --- a/tests/frontends/mpris/events_test.py +++ b/tests/frontends/mpris/events_test.py @@ -19,7 +19,7 @@ from tests import unittest class BackendEventsTest(unittest.TestCase): def setUp(self): # As a plain class, not an actor: - self.mpris_frontend = actor.MprisFrontend(core=None) + self.mpris_frontend = actor.MprisFrontend(config=None, core=None) self.mpris_object = mock.Mock(spec=objects.MprisObject) self.mpris_frontend.mpris_object = self.mpris_object diff --git a/tests/frontends/mpris/player_interface_test.py b/tests/frontends/mpris/player_interface_test.py index 0c477dc8..ec4a17a9 100644 --- a/tests/frontends/mpris/player_interface_test.py +++ b/tests/frontends/mpris/player_interface_test.py @@ -26,7 +26,7 @@ STOPPED = PlaybackState.STOPPED class PlayerInterfaceTest(unittest.TestCase): def setUp(self): objects.MprisObject._connect_to_dbus = mock.Mock() - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.mpris = objects.MprisObject(core=self.core) diff --git a/tests/frontends/mpris/playlists_interface_test.py b/tests/frontends/mpris/playlists_interface_test.py index 2adffaf3..745a858c 100644 --- a/tests/frontends/mpris/playlists_interface_test.py +++ b/tests/frontends/mpris/playlists_interface_test.py @@ -23,7 +23,7 @@ from tests import unittest class PlayerInterfaceTest(unittest.TestCase): def setUp(self): objects.MprisObject._connect_to_dbus = mock.Mock() - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.mpris = objects.MprisObject(core=self.core) diff --git a/tests/frontends/mpris/root_interface_test.py b/tests/frontends/mpris/root_interface_test.py index 722fd2cd..36d689a2 100644 --- a/tests/frontends/mpris/root_interface_test.py +++ b/tests/frontends/mpris/root_interface_test.py @@ -21,7 +21,7 @@ class RootInterfaceTest(unittest.TestCase): def setUp(self): objects.exit_process = mock.Mock() objects.MprisObject._connect_to_dbus = mock.Mock() - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.mpris = objects.MprisObject(core=self.core) diff --git a/tests/utils/jsonrpc_test.py b/tests/utils/jsonrpc_test.py index 226d4614..7fb8a55e 100644 --- a/tests/utils/jsonrpc_test.py +++ b/tests/utils/jsonrpc_test.py @@ -38,7 +38,7 @@ class Calculator(object): class JsonRpcTestBase(unittest.TestCase): def setUp(self): - self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.backend = dummy.create_dummy_backend_proxy() self.core = core.Core.start(backends=[self.backend]).proxy() self.jrw = jsonrpc.JsonRpcWrapper( From f8b22f32e70031ecd453a0b97f8dd2c1647853b7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 18:20:52 +0200 Subject: [PATCH 03/29] ext: Make info logging for loading extensions less verbose. --- mopidy/__main__.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 4f531000..86013885 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -138,7 +138,7 @@ def check_old_folders(): def load_extensions(): extensions = [] for entry_point in pkg_resources.iter_entry_points('mopidy.ext'): - logger.debug('Loading extension %s', entry_point.name) + logger.debug('Loading entrypoint: %s', entry_point) try: extension_class = entry_point.load() @@ -150,6 +150,9 @@ def load_extensions(): extension = extension_class() + logger.debug( + 'Loaded extension: %s %s', extension.dist_name, extension.version) + if entry_point.name != extension.ext_name: logger.warning( 'Disabled extension %(ep)s: entry point name (%(ep)s) ' @@ -164,14 +167,8 @@ def load_extensions(): 'Disabled extension %s: %s', entry_point.name, ex.message) continue - # TODO: due to order we do things in we can't know for sure if we are - # going to use it at this point, should we perhaps just log a single - # line with all extenions we found and then log an enabled line for - # each one after we check configs etc? - logger.info( - 'Loaded extension %s: %s %s', - entry_point.name, extension.dist_name, extension.version) extensions.append(extension) + logging.info('Loaded extensions: %s', ', '.join(e.ext_name for e in extensions)) return extensions From 91be3e379f3120fe94d33e3c3d34587a20f0bf6b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 18:21:18 +0200 Subject: [PATCH 04/29] config: Let user know what files we are loading. --- mopidy/__main__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 86013885..a7af9fb2 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -182,6 +182,8 @@ def load_config(options, extensions): # TODO Add config file given through `options` to `files` # TODO Replace `files` with single file given through `options` + logging.info('Loading config from: builtin-defaults, %s', ', '.join(files)) + # Read default core config parser.readfp(StringIO.StringIO(default_config)) @@ -222,7 +224,7 @@ def load_config(options, extensions): config[section_name] = schema.convert(parser.items(section_name)) except exceptions.ConfigError as error: for key in error: - logger.error('Config error: %s: %s', key, error[key]) + logger.error('Config error: %s:%s %s', section_name, key, error[key]) process.exit_process() return config From 7051c15539ca7f062fc509dfd80ecb622347cc2b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 18:38:07 +0200 Subject: [PATCH 05/29] config: Start reworking flow in main towards using new config. --- mopidy/__main__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index a7af9fb2..d18cd1a5 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -53,13 +53,14 @@ def main(): loop = gobject.MainLoop() options = parse_options() - config = {} # TODO: replace dummy placeholder try: - log.setup_logging(config, options.verbosity_level, options.save_debug_log) - check_old_folders() + # TODO: we need a two stage logging setup as we want logging for + # extension loading and config loading. + log.setup_logging(None, options.verbosity_level, options.save_debug_log) extensions = load_extensions() - load_config(options, extensions) + config = load_config(options, extensions) + check_old_folders() setup_settings(options.interactive) audio = setup_audio(config) backends = setup_backends(config, extensions, audio) From 47570fcddc7555519c382e9738aea3f776487fda Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 18:58:48 +0200 Subject: [PATCH 06/29] config: Split load and validate config into two. --- mopidy/__main__.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index d18cd1a5..c3c627e4 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -59,7 +59,8 @@ def main(): # extension loading and config loading. log.setup_logging(None, options.verbosity_level, options.save_debug_log) extensions = load_extensions() - config = load_config(options, extensions) + raw_config = load_config(options, extensions) + config = validate_config(raw_config, extensions) check_old_folders() setup_settings(options.interactive) audio = setup_audio(config) @@ -169,7 +170,9 @@ def load_extensions(): continue extensions.append(extension) - logging.info('Loaded extensions: %s', ', '.join(e.ext_name for e in extensions)) + + names = (e.ext_name for e in extensions) + logging.info('Found following runnable extensions: %s', ', '.join(names)) return extensions @@ -205,24 +208,31 @@ def load_config(options, extensions): logger.error('Config file %s is not UTF-8 encoded', filename) process.exit_process() - # TODO Merge config values given through `options` into `config` + raw_config = {} + for section in parser.sections(): + raw_config[section] = dict(parser.items(section)) + # TODO Merge config values given through `options` into `raw_config` + return raw_config + + +def validate_config(raw_config, extensions): # Collect config schemas to validate against sections_and_schemas = config_schemas.items() for extension in extensions: section_name = 'ext.%s' % extension.ext_name - if parser.getboolean(section_name, 'enabled'): - sections_and_schemas.append( - (section_name, extension.get_config_schema())) + sections_and_schemas.append( + (section_name, extension.get_config_schema())) # Get validated config config = {} for section_name, schema in sections_and_schemas: - if not parser.has_section(section_name): + if section_name not in raw_config: logger.error('Config section %s not found', section_name) process.exit_process() try: - config[section_name] = schema.convert(parser.items(section_name)) + items = raw_config[section_name].items() + config[section_name] = schema.convert(items) except exceptions.ConfigError as error: for key in error: logger.error('Config error: %s:%s %s', section_name, key, error[key]) From 7c124d0f7285b7661b7f1068e8d3d18fab6b4e41 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 19:00:00 +0200 Subject: [PATCH 07/29] config/ext: fitler enabled extensions. --- mopidy/__main__.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index c3c627e4..020252c9 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -41,7 +41,8 @@ from mopidy.audio import Audio from mopidy.config import default_config, config_schemas from mopidy.core import Core from mopidy.utils import ( - deps, log, path, process, settings as settings_utils, versioning) + config as config_utils, deps, log, path, process, + settings as settings_utils, versioning) logger = logging.getLogger('mopidy.main') @@ -60,6 +61,7 @@ def main(): log.setup_logging(None, options.verbosity_level, options.save_debug_log) extensions = load_extensions() raw_config = load_config(options, extensions) + extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, extensions) check_old_folders() setup_settings(options.interactive) @@ -176,6 +178,21 @@ def load_extensions(): return extensions +def filter_enabled_extensions(raw_config, extensions): + boolean = config_utils.Boolean() + filtered_extensions = [] + + for extension in extensions: + # TODO: handle key and value errors. + enabled = raw_config['ext.%s' % extension.ext_name]['enabled'] + if boolean.deserialize(enabled): + filtered_extensions.append(extension) + + names = (e.ext_name for e in filtered_extensions) + logging.info('Following extensions will be started: %s', ', '.join(names)) + return filtered_extensions + + def load_config(options, extensions): parser = ConfigParser.RawConfigParser() From dcd0d7e8136b0b30fdd230709dcdeb48d0f776b7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 19:07:17 +0200 Subject: [PATCH 08/29] ext: Rename ext_name to name and provide ext_name property that gives ext.$name --- mopidy/__main__.py | 13 ++++++------- mopidy/backends/local/__init__.py | 2 +- mopidy/backends/spotify/__init__.py | 2 +- mopidy/backends/stream/__init__.py | 2 +- mopidy/ext.py | 8 +++++++- mopidy/frontends/http/__init__.py | 2 +- mopidy/frontends/lastfm/__init__.py | 2 +- mopidy/frontends/mpd/__init__.py | 2 +- mopidy/frontends/mpris/__init__.py | 2 +- tests/ext_test.py | 7 +++++++ 10 files changed, 27 insertions(+), 15 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 020252c9..dc5d8d8d 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -157,11 +157,11 @@ def load_extensions(): logger.debug( 'Loaded extension: %s %s', extension.dist_name, extension.version) - if entry_point.name != extension.ext_name: + if entry_point.name != extension.name: logger.warning( 'Disabled extension %(ep)s: entry point name (%(ep)s) ' 'does not match extension name (%(ext)s)', - {'ep': entry_point.name, 'ext': extension.ext_name}) + {'ep': entry_point.name, 'ext': extension.name}) continue try: @@ -173,7 +173,7 @@ def load_extensions(): extensions.append(extension) - names = (e.ext_name for e in extensions) + names = (e.name for e in extensions) logging.info('Found following runnable extensions: %s', ', '.join(names)) return extensions @@ -184,11 +184,11 @@ def filter_enabled_extensions(raw_config, extensions): for extension in extensions: # TODO: handle key and value errors. - enabled = raw_config['ext.%s' % extension.ext_name]['enabled'] + enabled = raw_config[extension.ext_name]['enabled'] if boolean.deserialize(enabled): filtered_extensions.append(extension) - names = (e.ext_name for e in filtered_extensions) + names = (e.name for e in filtered_extensions) logging.info('Following extensions will be started: %s', ', '.join(names)) return filtered_extensions @@ -237,9 +237,8 @@ def validate_config(raw_config, extensions): # Collect config schemas to validate against sections_and_schemas = config_schemas.items() for extension in extensions: - section_name = 'ext.%s' % extension.ext_name sections_and_schemas.append( - (section_name, extension.get_config_schema())) + (extension.ext_name, extension.get_config_schema())) # Get validated config config = {} diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index bd9a6d1a..a93c8fa8 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -47,7 +47,7 @@ None class Extension(ext.Extension): dist_name = 'Mopidy-Local' - ext_name = 'local' + name = 'local' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/backends/spotify/__init__.py b/mopidy/backends/spotify/__init__.py index d9fdb630..a19d7ea8 100644 --- a/mopidy/backends/spotify/__init__.py +++ b/mopidy/backends/spotify/__init__.py @@ -68,7 +68,7 @@ https://github.com/mopidy/mopidy/issues?labels=Spotify+backend class Extension(ext.Extension): dist_name = 'Mopidy-Spotify' - ext_name = 'spotify' + name = 'spotify' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/backends/stream/__init__.py b/mopidy/backends/stream/__init__.py index d14275b0..321e4a03 100644 --- a/mopidy/backends/stream/__init__.py +++ b/mopidy/backends/stream/__init__.py @@ -46,7 +46,7 @@ None class Extension(ext.Extension): dist_name = 'Mopidy-Stream' - ext_name = 'stream' + name = 'stream' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/ext.py b/mopidy/ext.py index bc26069c..78283617 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -6,9 +6,15 @@ from mopidy.utils import config class Extension(object): dist_name = None - ext_name = None + name = None version = None + @property + def ext_name(self): + if self.name is None: + return None + return 'ext.%s' % self.name + def get_default_config(self): raise NotImplementedError( 'Add at least a config section with "enabled = true"') diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index 5e9629a7..d2fe24fc 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -520,7 +520,7 @@ Example to get started with class Extension(ext.Extension): dist_name = 'Mopidy-HTTP' - ext_name = 'http' + name = 'http' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/frontends/lastfm/__init__.py b/mopidy/frontends/lastfm/__init__.py index 510856ea..7a252170 100644 --- a/mopidy/frontends/lastfm/__init__.py +++ b/mopidy/frontends/lastfm/__init__.py @@ -45,7 +45,7 @@ The frontend is enabled by default if all dependencies are available. class Extension(ext.Extension): dist_name = 'Mopidy-Lastfm' - ext_name = 'lastfm' + name = 'lastfm' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index 518da54a..d782c545 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -89,7 +89,7 @@ near future: class Extension(ext.Extension): dist_name = 'Mopidy-MPD' - ext_name = 'mpd' + name = 'mpd' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/frontends/mpris/__init__.py b/mopidy/frontends/mpris/__init__.py index 20ef0ea7..4b817c73 100644 --- a/mopidy/frontends/mpris/__init__.py +++ b/mopidy/frontends/mpris/__init__.py @@ -71,7 +71,7 @@ Now you can control Mopidy through the player object. Examples: class Extension(ext.Extension): dist_name = 'Mopidy-MPRIS' - ext_name = 'mpris' + name = 'mpris' version = mopidy.__version__ def get_default_config(self): diff --git a/tests/ext_test.py b/tests/ext_test.py index b58333c2..b967f951 100644 --- a/tests/ext_test.py +++ b/tests/ext_test.py @@ -13,9 +13,16 @@ class ExtensionTest(unittest.TestCase): def test_dist_name_is_none(self): self.assertIsNone(self.ext.dist_name) + def test_name_is_none(self): + self.assertIsNone(self.ext.name) + def test_ext_name_is_none(self): self.assertIsNone(self.ext.ext_name) + def test_ext_name_prefixes_ext(self): + self.ext.name = 'foo' + self.assertEqual('ext.foo', self.ext.ext_name) + def test_version_is_none(self): self.assertIsNone(self.ext.version) From 29a4ff040e79d19e908ee6bda5e0e5ceabe81621 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 19:19:24 +0200 Subject: [PATCH 09/29] config: Make list return tuples so we get imuttable data. --- mopidy/utils/config.py | 2 +- tests/utils/config_test.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mopidy/utils/config.py b/mopidy/utils/config.py index 6358796b..7bc5ebe1 100644 --- a/mopidy/utils/config.py +++ b/mopidy/utils/config.py @@ -154,7 +154,7 @@ class List(ConfigValue): values = re.split(r'\s*\n\s*', value.strip()) else: values = re.split(r'\s*,\s*', value.strip()) - return [v for v in values if v] + return tuple([v for v in values if v]) def serialize(self, value): return '\n '.join(v.encode('utf-8') for v in value) diff --git a/tests/utils/config_test.py b/tests/utils/config_test.py index 43bb1679..9db2922e 100644 --- a/tests/utils/config_test.py +++ b/tests/utils/config_test.py @@ -199,10 +199,10 @@ class ListTest(unittest.TestCase): def test_deserialize_conversion_success(self): value = config.List() - expected = ['foo', 'bar', 'baz'] + expected = ('foo', 'bar', 'baz') self.assertEqual(expected, value.deserialize('foo, bar ,baz ')) - expected = ['foo,bar', 'bar', 'baz'] + expected = ('foo,bar', 'bar', 'baz') self.assertEqual(expected, value.deserialize(' foo,bar\nbar\nbaz')) def test_deserialize_enforces_required(self): @@ -212,12 +212,12 @@ class ListTest(unittest.TestCase): def test_deserialize_respects_optional(self): value = config.List(optional=True) - self.assertEqual([], value.deserialize('')) - self.assertEqual([], value.deserialize(' ')) + self.assertEqual(tuple(), value.deserialize('')) + self.assertEqual(tuple(), value.deserialize(' ')) def test_serialize(self): value = config.List() - result = value.serialize(['foo', 'bar', 'baz']) + result = value.serialize(('foo', 'bar', 'baz')) self.assertRegexpMatches(result, r'foo\n\s*bar\n\s*baz') From 77cdb5b0658afe0751b1e9debc06210838d8eb01 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 19:33:26 +0200 Subject: [PATCH 10/29] config: Add config based list_settings_callback --- mopidy/__main__.py | 21 ++++++++++++++++++++- mopidy/utils/config.py | 6 +----- tests/utils/config_test.py | 4 ---- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index dc5d8d8d..b6c797fd 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -113,7 +113,7 @@ def parse_options(): parser.add_option( b'--list-settings', action='callback', - callback=settings_utils.list_settings_optparse_callback, + callback=list_settings_callback, help='list current settings') parser.add_option( b'--list-deps', @@ -126,6 +126,25 @@ def parse_options(): return parser.parse_args(args=mopidy_args)[0] +def list_settings_callback(options, opt, value, parser): + extensions = load_extensions() + raw_config = load_config(options, extensions) + extensions = filter_enabled_extensions(raw_config, extensions) + config = validate_config(raw_config, extensions) + + # TODO: this code is duplicated, figure out a way to reuse it? + sections_and_schemas = config_schemas.items() + for extension in extensions: + sections_and_schemas.append( + (extension.ext_name, extension.get_config_schema())) + + output = ['# Settings for disabled extensions are not shown.'] + for section_name, schema in sections_and_schemas: + output.append(schema.format(section_name, config.get(section_name, {}))) + print '\n\n'.join(output) + sys.exit(0) + + def check_old_folders(): # TODO: add old settings and pre extension storage locations? old_settings_folder = os.path.expanduser('~/.mopidy') diff --git a/mopidy/utils/config.py b/mopidy/utils/config.py index 7bc5ebe1..00f2b595 100644 --- a/mopidy/utils/config.py +++ b/mopidy/utils/config.py @@ -157,7 +157,7 @@ class List(ConfigValue): return tuple([v for v in values if v]) def serialize(self, value): - return '\n '.join(v.encode('utf-8') for v in value) + return '\n ' + '\n '.join(v.encode('utf-8') for v in value) class LogLevel(ConfigValue): @@ -270,10 +270,6 @@ class ExtensionConfigSchema(ConfigSchema): super(ExtensionConfigSchema, self).__init__() self['enabled'] = Boolean() - def format(self, name, values): - return super(ExtensionConfigSchema, self).format( - 'ext.%s' % name, values) - class LogLevelConfigSchema(object): """Special cased schema for handling a config section with loglevels. diff --git a/tests/utils/config_test.py b/tests/utils/config_test.py index 9db2922e..f2465b4e 100644 --- a/tests/utils/config_test.py +++ b/tests/utils/config_test.py @@ -376,10 +376,6 @@ class ExtensionConfigSchemaTest(unittest.TestCase): schema = config.ExtensionConfigSchema() self.assertIsInstance(schema['enabled'], config.Boolean) - def test_section_name_is_prefixed(self): - schema = config.ExtensionConfigSchema() - self.assertEqual('[ext.foo]', schema.format('foo', {})) - class LogLevelConfigSchemaTest(unittest.TestCase): def test_conversion(self): From 3b8fe2fd9fff117ebb6cfa40bab635315796db3a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 19:37:51 +0200 Subject: [PATCH 11/29] config: Remove old list settings and mask secret code. --- mopidy/utils/settings.py | 36 --------------------- tests/utils/settings_test.py | 62 ------------------------------------ 2 files changed, 98 deletions(-) diff --git a/mopidy/utils/settings.py b/mopidy/utils/settings.py index 051f0f1c..b61476f5 100644 --- a/mopidy/utils/settings.py +++ b/mopidy/utils/settings.py @@ -184,42 +184,6 @@ def validate_settings(defaults, settings): return errors -def list_settings_optparse_callback(*args): - """ - Prints a list of all settings. - - Called by optparse when Mopidy is run with the :option:`--list-settings` - option. - """ - from mopidy import settings - print format_settings_list(settings) - sys.exit(0) - - -def format_settings_list(settings): - errors = settings.get_errors() - lines = [] - for (key, value) in sorted(settings.current.iteritems()): - default_value = settings.default.get(key) - masked_value = mask_value_if_secret(key, value) - lines.append('%s: %s' % ( - key, formatting.indent(pprint.pformat(masked_value), places=2))) - if value != default_value and default_value is not None: - lines.append( - ' Default: %s' % - formatting.indent(pprint.pformat(default_value), places=4)) - if errors.get(key) is not None: - lines.append(' Error: %s' % errors[key]) - return '\n'.join(lines) - - -def mask_value_if_secret(key, value): - if key.endswith('PASSWORD') and value: - return '********' - else: - return value - - def did_you_mean(setting, defaults): """Suggest most likely setting based on levenshtein.""" if not defaults: diff --git a/tests/utils/settings_test.py b/tests/utils/settings_test.py index 2c13066c..787337d2 100644 --- a/tests/utils/settings_test.py +++ b/tests/utils/settings_test.py @@ -59,20 +59,6 @@ class ValidateSettingsTest(unittest.TestCase): self.defaults, {'FOO': '', 'BAR': ''}) self.assertEqual(len(result), 2) - def test_masks_value_if_secret(self): - secret = setting_utils.mask_value_if_secret('SPOTIFY_PASSWORD', 'bar') - self.assertEqual('********', secret) - - def test_does_not_mask_value_if_not_secret(self): - not_secret = setting_utils.mask_value_if_secret( - 'SPOTIFY_USERNAME', 'foo') - self.assertEqual('foo', not_secret) - - def test_does_not_mask_value_if_none(self): - not_secret = setting_utils.mask_value_if_secret( - 'SPOTIFY_USERNAME', None) - self.assertEqual(None, not_secret) - class SettingsProxyTest(unittest.TestCase): def setUp(self): @@ -179,54 +165,6 @@ class SettingsProxyTest(unittest.TestCase): self.settings.validate(interactive=True) -class FormatSettingListTest(unittest.TestCase): - def setUp(self): - self.settings = setting_utils.SettingsProxy(settings) - - def test_contains_the_setting_name(self): - self.settings.TEST = 'test' - result = setting_utils.format_settings_list(self.settings) - self.assertIn('TEST:', result, result) - - def test_repr_of_a_string_value(self): - self.settings.TEST = 'test' - result = setting_utils.format_settings_list(self.settings) - self.assertIn("TEST: u'test'", result, result) - - def test_repr_of_an_int_value(self): - self.settings.TEST = 123 - result = setting_utils.format_settings_list(self.settings) - self.assertIn("TEST: 123", result, result) - - def test_repr_of_a_tuple_value(self): - self.settings.TEST = (123, 'abc') - result = setting_utils.format_settings_list(self.settings) - self.assertIn("TEST: (123, u'abc')", result, result) - - def test_passwords_are_masked(self): - self.settings.TEST_PASSWORD = 'secret' - result = setting_utils.format_settings_list(self.settings) - self.assertNotIn("TEST_PASSWORD: u'secret'", result, result) - self.assertIn("TEST_PASSWORD: u'********'", result, result) - - def test_short_values_are_not_pretty_printed(self): - self.settings.FRONTEND = ('mopidy.frontends.mpd.MpdFrontend',) - result = setting_utils.format_settings_list(self.settings) - self.assertIn( - "FRONTEND: (u'mopidy.frontends.mpd.MpdFrontend',)", result) - - def test_long_values_are_pretty_printed(self): - self.settings.FRONTEND = ( - u'mopidy.frontends.mpd.MpdFrontend', - u'mopidy.frontends.lastfm.LastfmFrontend') - result = setting_utils.format_settings_list(self.settings) - self.assertIn( - "FRONTEND: \n" - " (u'mopidy.frontends.mpd.MpdFrontend',\n" - " u'mopidy.frontends.lastfm.LastfmFrontend')", - result) - - class DidYouMeanTest(unittest.TestCase): def testSuggestoins(self): defaults = { From 15d0c7a13fdb4e48aacde1bab834b3a19a5f2ab0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 20:10:35 +0200 Subject: [PATCH 12/29] config: Merge command line overrides into config. --- mopidy/__main__.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index b6c797fd..60c0d73d 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -85,9 +85,24 @@ def main(): process.stop_remaining_actors() +def check_config_override(option, opt, override): + try: + section, remainder = override.split(':', 1) + key, value = remainder.split('=', 1) + return (section, key, value) + except ValueError: + raise optparse.OptionValueError( + 'option %s: must have the format section:key=value' % opt) + + def parse_options(): parser = optparse.OptionParser( version='Mopidy %s' % versioning.get_version()) + + # Ugly extension of optparse type checking magic :/ + optparse.Option.TYPES += ('setting',) + optparse.Option.TYPE_CHECKER['setting'] = check_config_override + # NOTE First argument to add_option must be bytestrings on Python < 2.6.2 # See https://github.com/mopidy/mopidy/issues/302 for details parser.add_option( @@ -112,8 +127,7 @@ def parse_options(): help='save debug log to "./mopidy.log"') parser.add_option( b'--list-settings', - action='callback', - callback=list_settings_callback, + action='callback', callback=list_settings_callback, help='list current settings') parser.add_option( b'--list-deps', @@ -123,12 +137,16 @@ def parse_options(): b'--debug-thread', action='store_true', dest='debug_thread', help='run background thread that dumps tracebacks on SIGUSR1') + parser.add_option( + b'-s', b'--setting', + action='append', dest='settings', type='setting', + help='`section_name:setting_key=value` values to override settings.') return parser.parse_args(args=mopidy_args)[0] -def list_settings_callback(options, opt, value, parser): +def list_settings_callback(option, opt, value, parser): extensions = load_extensions() - raw_config = load_config(options, extensions) + raw_config = load_config(parser.values, extensions) extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, extensions) @@ -221,8 +239,10 @@ def load_config(options, extensions): ] # TODO Add config file given through `options` to `files` # TODO Replace `files` with single file given through `options` + # TODO expand_path and use xdg when loading. - logging.info('Loading config from: builtin-defaults, %s', ', '.join(files)) + sources = ['builtin-defaults'] + files + ['command-line'] + logging.info('Loading config from: %s', ', '.join(sources)) # Read default core config parser.readfp(StringIO.StringIO(default_config)) @@ -248,7 +268,9 @@ def load_config(options, extensions): for section in parser.sections(): raw_config[section] = dict(parser.items(section)) - # TODO Merge config values given through `options` into `raw_config` + for section, key, value in options.settings or []: + raw_config.setdefault(section, {})[key] = value + return raw_config From e226ddd6529fbc5461e9fab640cbc66789f5a83d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 22:12:00 +0200 Subject: [PATCH 13/29] config/ext: Remove ext. prefix from configs. --- docs/extensiondev.rst | 18 +++++++++--------- mopidy/__main__.py | 8 ++++---- mopidy/backends/local/__init__.py | 4 ++-- mopidy/backends/spotify/__init__.py | 4 ++-- mopidy/backends/stream/__init__.py | 4 ++-- mopidy/ext.py | 8 +------- mopidy/frontends/http/__init__.py | 4 ++-- mopidy/frontends/lastfm/__init__.py | 4 ++-- mopidy/frontends/mpd/__init__.py | 4 ++-- mopidy/frontends/mpris/__init__.py | 4 ++-- mopidy/utils/config.py | 3 +-- tests/ext_test.py | 7 ------- 12 files changed, 29 insertions(+), 43 deletions(-) diff --git a/docs/extensiondev.rst b/docs/extensiondev.rst index 71ea9ec0..b64ac3b6 100644 --- a/docs/extensiondev.rst +++ b/docs/extensiondev.rst @@ -99,7 +99,7 @@ installation using ``pip install Mopidy-Something==dev`` to work. Before starting Mopidy, you must add your Soundspot username and password to the Mopidy configuration file:: - [ext.soundspot] + [soundspot] username = alice password = secret @@ -199,13 +199,13 @@ The default configuration for the extension is defined by the ``get_default_config()`` method in the ``Extension`` class which returns a :mod:`ConfigParser` compatible config section. The config section's name should be the same as the extension's short name, as defined in the ``entry_points`` -part of ``setup.py``, but prefixed with ``ext.``, for example -``ext.soundspot``. All extensions should include an ``enabled`` config which -should default to ``true``. Provide good defaults for all config values so that -as few users as possible will need to change them. The exception is if the -config value has security implications; in that case you should default to the -most secure configuration. Leave any configurations that doesn't have -meaningful defaults blank, like ``username`` and ``password``. +part of ``setup.py``, for example ``soundspot``. All extensions should include +an ``enabled`` config which should default to ``true``. Provide good defaults +for all config values so that as few users as possible will need to change +them. The exception is if the config value has security implications; in that +case you should default to the most secure configuration. Leave any +configurations that doesn't have meaningful defaults blank, like ``username`` +and ``password``. :: @@ -225,7 +225,7 @@ meaningful defaults blank, like ``username`` and ``password``. __version__ = '0.1' default_config = """ - [ext.soundspot] + [soundspot] enabled = true username = password = diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 60c0d73d..5c2f7cfd 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -194,11 +194,11 @@ def load_extensions(): logger.debug( 'Loaded extension: %s %s', extension.dist_name, extension.version) - if entry_point.name != extension.name: + if entry_point.name != extension.ext_name: logger.warning( 'Disabled extension %(ep)s: entry point name (%(ep)s) ' 'does not match extension name (%(ext)s)', - {'ep': entry_point.name, 'ext': extension.name}) + {'ep': entry_point.name, 'ext': extension.ext_name}) continue try: @@ -210,7 +210,7 @@ def load_extensions(): extensions.append(extension) - names = (e.name for e in extensions) + names = (e.ext_name for e in extensions) logging.info('Found following runnable extensions: %s', ', '.join(names)) return extensions @@ -225,7 +225,7 @@ def filter_enabled_extensions(raw_config, extensions): if boolean.deserialize(enabled): filtered_extensions.append(extension) - names = (e.name for e in filtered_extensions) + names = (e.ext_name for e in filtered_extensions) logging.info('Following extensions will be started: %s', ', '.join(names)) return filtered_extensions diff --git a/mopidy/backends/local/__init__.py b/mopidy/backends/local/__init__.py index a93c8fa8..17fd659e 100644 --- a/mopidy/backends/local/__init__.py +++ b/mopidy/backends/local/__init__.py @@ -6,7 +6,7 @@ from mopidy.utils import config, formatting default_config = """ -[ext.local] +[local] # If the local extension should be enabled or not enabled = true @@ -47,7 +47,7 @@ None class Extension(ext.Extension): dist_name = 'Mopidy-Local' - name = 'local' + ext_name = 'local' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/backends/spotify/__init__.py b/mopidy/backends/spotify/__init__.py index a19d7ea8..0e32d4cd 100644 --- a/mopidy/backends/spotify/__init__.py +++ b/mopidy/backends/spotify/__init__.py @@ -7,7 +7,7 @@ from mopidy.utils import config, formatting default_config = """ -[ext.spotify] +[spotify] # If the Spotify extension should be enabled or not enabled = true @@ -68,7 +68,7 @@ https://github.com/mopidy/mopidy/issues?labels=Spotify+backend class Extension(ext.Extension): dist_name = 'Mopidy-Spotify' - name = 'spotify' + ext_name = 'spotify' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/backends/stream/__init__.py b/mopidy/backends/stream/__init__.py index 321e4a03..9a393bed 100644 --- a/mopidy/backends/stream/__init__.py +++ b/mopidy/backends/stream/__init__.py @@ -6,7 +6,7 @@ from mopidy.utils import config, formatting default_config = """ -[ext.stream] +[stream] # If the stream extension should be enabled or not enabled = true @@ -46,7 +46,7 @@ None class Extension(ext.Extension): dist_name = 'Mopidy-Stream' - name = 'stream' + ext_name = 'stream' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/ext.py b/mopidy/ext.py index 78283617..bc26069c 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -6,15 +6,9 @@ from mopidy.utils import config class Extension(object): dist_name = None - name = None + ext_name = None version = None - @property - def ext_name(self): - if self.name is None: - return None - return 'ext.%s' % self.name - def get_default_config(self): raise NotImplementedError( 'Add at least a config section with "enabled = true"') diff --git a/mopidy/frontends/http/__init__.py b/mopidy/frontends/http/__init__.py index d2fe24fc..03bf0a87 100644 --- a/mopidy/frontends/http/__init__.py +++ b/mopidy/frontends/http/__init__.py @@ -6,7 +6,7 @@ from mopidy.utils import config, formatting default_config = """ -[ext.http] +[http] # If the HTTP extension should be enabled or not enabled = true @@ -520,7 +520,7 @@ Example to get started with class Extension(ext.Extension): dist_name = 'Mopidy-HTTP' - name = 'http' + ext_name = 'http' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/frontends/lastfm/__init__.py b/mopidy/frontends/lastfm/__init__.py index 7a252170..f4bff0e5 100644 --- a/mopidy/frontends/lastfm/__init__.py +++ b/mopidy/frontends/lastfm/__init__.py @@ -6,7 +6,7 @@ from mopidy.utils import config, formatting default_config = """ -[ext.lastfm] +[lastfm] # If the Last.fm extension should be enabled or not enabled = true @@ -45,7 +45,7 @@ The frontend is enabled by default if all dependencies are available. class Extension(ext.Extension): dist_name = 'Mopidy-Lastfm' - name = 'lastfm' + ext_name = 'lastfm' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/frontends/mpd/__init__.py b/mopidy/frontends/mpd/__init__.py index d782c545..08bafd26 100644 --- a/mopidy/frontends/mpd/__init__.py +++ b/mopidy/frontends/mpd/__init__.py @@ -6,7 +6,7 @@ from mopidy.utils import config, formatting default_config = """ -[ext.mpd] +[mpd] # If the MPD extension should be enabled or not enabled = true @@ -89,7 +89,7 @@ near future: class Extension(ext.Extension): dist_name = 'Mopidy-MPD' - name = 'mpd' + ext_name = 'mpd' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/frontends/mpris/__init__.py b/mopidy/frontends/mpris/__init__.py index 4b817c73..79806c47 100644 --- a/mopidy/frontends/mpris/__init__.py +++ b/mopidy/frontends/mpris/__init__.py @@ -6,7 +6,7 @@ from mopidy.utils import formatting, config default_config = """ -[ext.mpris] +[mpris] # If the MPRIS extension should be enabled or not enabled = true @@ -71,7 +71,7 @@ Now you can control Mopidy through the player object. Examples: class Extension(ext.Extension): dist_name = 'Mopidy-MPRIS' - name = 'mpris' + ext_name = 'mpris' version = mopidy.__version__ def get_default_config(self): diff --git a/mopidy/utils/config.py b/mopidy/utils/config.py index 00f2b595..d2b34e7d 100644 --- a/mopidy/utils/config.py +++ b/mopidy/utils/config.py @@ -263,8 +263,7 @@ class ConfigSchema(object): class ExtensionConfigSchema(ConfigSchema): """Sub-classed :class:`ConfigSchema` for use in extensions. - Ensures that `enabled` config value is present and that section name is - prefixed with ext. + Ensures that `enabled` config value is present. """ def __init__(self): super(ExtensionConfigSchema, self).__init__() diff --git a/tests/ext_test.py b/tests/ext_test.py index b967f951..b58333c2 100644 --- a/tests/ext_test.py +++ b/tests/ext_test.py @@ -13,16 +13,9 @@ class ExtensionTest(unittest.TestCase): def test_dist_name_is_none(self): self.assertIsNone(self.ext.dist_name) - def test_name_is_none(self): - self.assertIsNone(self.ext.name) - def test_ext_name_is_none(self): self.assertIsNone(self.ext.ext_name) - def test_ext_name_prefixes_ext(self): - self.ext.name = 'foo' - self.assertEqual('ext.foo', self.ext.ext_name) - def test_version_is_none(self): self.assertIsNone(self.ext.version) From 0b416bc892f64d1506951085c178648ebda8fa97 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 22:19:12 +0200 Subject: [PATCH 14/29] config: Update override format and flag name. --- mopidy/__main__.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 5c2f7cfd..ce615b11 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -54,13 +54,14 @@ def main(): loop = gobject.MainLoop() options = parse_options() + config_overrides = getattr(options, 'overrides', []) try: # TODO: we need a two stage logging setup as we want logging for # extension loading and config loading. log.setup_logging(None, options.verbosity_level, options.save_debug_log) extensions = load_extensions() - raw_config = load_config(options, extensions) + raw_config = load_config(config_overrides, extensions) extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, extensions) check_old_folders() @@ -87,12 +88,12 @@ def main(): def check_config_override(option, opt, override): try: - section, remainder = override.split(':', 1) + section, remainder = override.split('/', 1) key, value = remainder.split('=', 1) return (section, key, value) except ValueError: raise optparse.OptionValueError( - 'option %s: must have the format section:key=value' % opt) + 'option %s: must have the format section/key=value' % opt) def parse_options(): @@ -100,8 +101,8 @@ def parse_options(): version='Mopidy %s' % versioning.get_version()) # Ugly extension of optparse type checking magic :/ - optparse.Option.TYPES += ('setting',) - optparse.Option.TYPE_CHECKER['setting'] = check_config_override + optparse.Option.TYPES += ('config_override',) + optparse.Option.TYPE_CHECKER['config_override'] = check_config_override # NOTE First argument to add_option must be bytestrings on Python < 2.6.2 # See https://github.com/mopidy/mopidy/issues/302 for details @@ -138,15 +139,17 @@ def parse_options(): action='store_true', dest='debug_thread', help='run background thread that dumps tracebacks on SIGUSR1') parser.add_option( - b'-s', b'--setting', - action='append', dest='settings', type='setting', - help='`section_name:setting_key=value` values to override settings.') + b'-o', b'--option', + action='append', dest='overrides', type='config_override', + help='`section/key=value` values to override config options.') return parser.parse_args(args=mopidy_args)[0] def list_settings_callback(option, opt, value, parser): + overrides = getattr(parser.values, 'overrides', []) + extensions = load_extensions() - raw_config = load_config(parser.values, extensions) + raw_config = load_config(overrides, extensions) extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, extensions) @@ -268,7 +271,7 @@ def load_config(options, extensions): for section in parser.sections(): raw_config[section] = dict(parser.items(section)) - for section, key, value in options.settings or []: + for section, key, value in options or []: raw_config.setdefault(section, {})[key] = value return raw_config From 3f59e16f7c32c92cbd34dd5ae546be1450f4a070 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 22:26:03 +0200 Subject: [PATCH 15/29] config: Collect all config errors on startup. --- mopidy/__main__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index ce615b11..102dec8c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -286,6 +286,7 @@ def validate_config(raw_config, extensions): # Get validated config config = {} + errors = {} for section_name, schema in sections_and_schemas: if section_name not in raw_config: logger.error('Config section %s not found', section_name) @@ -294,9 +295,14 @@ def validate_config(raw_config, extensions): items = raw_config[section_name].items() config[section_name] = schema.convert(items) except exceptions.ConfigError as error: + errors[section_name] = error + + if errors: + for section_name, error in errors.items(): + logger.error('[%s] config errors:', section_name) for key in error: - logger.error('Config error: %s:%s %s', section_name, key, error[key]) - process.exit_process() + logger.error('%s %s', key, error[key]) + process.exit_process() return config From f73c081ddfeaec5875b9bf3bdcdfa3c55afc3403 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 22:36:22 +0200 Subject: [PATCH 16/29] config: Improve list settings output with disabled settings. --- mopidy/__main__.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 102dec8c..99d6cf2a 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -150,18 +150,24 @@ def list_settings_callback(option, opt, value, parser): extensions = load_extensions() raw_config = load_config(overrides, extensions) - extensions = filter_enabled_extensions(raw_config, extensions) - config = validate_config(raw_config, extensions) - - # TODO: this code is duplicated, figure out a way to reuse it? - sections_and_schemas = config_schemas.items() - for extension in extensions: - sections_and_schemas.append( - (extension.ext_name, extension.get_config_schema())) + enabled_extensions = filter_enabled_extensions(raw_config, extensions) + config = validate_config(raw_config, enabled_extensions) output = ['# Settings for disabled extensions are not shown.'] - for section_name, schema in sections_and_schemas: - output.append(schema.format(section_name, config.get(section_name, {}))) + for section_name, schema in config_schemas.items(): + options = config.get(section_name, {}) + if not options: + continue + output.append(schema.format(section_name, options)) + + for extension in extensions: + if extension in enabled_extensions: + schema = extension.get_config_schema() + options = config.get(extension.ext_name, {}) + output.append(schema.format(extension.ext_name, options)) + else: + output.append('[%s]\nenabled = false' % extension.ext_name) + print '\n\n'.join(output) sys.exit(0) From 3c14d09c87817dc96c12ff74ceddf46dd8bab914 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 22:51:40 +0200 Subject: [PATCH 17/29] config: Rename ConfigParser to configparser. --- mopidy/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 99d6cf2a..78968dc7 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import codecs -import ConfigParser +import ConfigParser as configparser import logging import optparse import os @@ -240,7 +240,7 @@ def filter_enabled_extensions(raw_config, extensions): def load_config(options, extensions): - parser = ConfigParser.RawConfigParser() + parser = configparser.RawConfigParser() files = [ '/etc/mopidy/mopidy.conf', From 9310c08f4d3387dbfb8b12162ff2d904383a3c25 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 22:56:01 +0200 Subject: [PATCH 18/29] config: Use path.expand_path and $XDG_CONFIG_DIR instead of hardcoded .config --- mopidy/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 78968dc7..16ef61bc 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -244,7 +244,7 @@ def load_config(options, extensions): files = [ '/etc/mopidy/mopidy.conf', - '~/.config/mopidy/mopidy.conf', + '$XDG_CONFIG_DIR/mopidy/mopidy.conf', ] # TODO Add config file given through `options` to `files` # TODO Replace `files` with single file given through `options` @@ -262,7 +262,7 @@ def load_config(options, extensions): # Load config from a series of config files for filename in files: - filename = os.path.expanduser(filename) + filename = path.expand_path(filename) try: filehandle = codecs.open(filename, encoding='utf-8') parser.readfp(filehandle) From 4802d8033398b675d4dbf5a326b8eb1e6cfcb381 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:03:22 +0200 Subject: [PATCH 19/29] config: Rename --list-settings to --show-config --- mopidy/__main__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 16ef61bc..258c1cfd 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -127,9 +127,9 @@ def parse_options(): action='store_true', dest='save_debug_log', help='save debug log to "./mopidy.log"') parser.add_option( - b'--list-settings', - action='callback', callback=list_settings_callback, - help='list current settings') + b'--show-config', + action='callback', callback=show_config_callback, + help='show current config') parser.add_option( b'--list-deps', action='callback', callback=deps.list_deps_optparse_callback, @@ -145,7 +145,7 @@ def parse_options(): return parser.parse_args(args=mopidy_args)[0] -def list_settings_callback(option, opt, value, parser): +def show_config_callback(option, opt, value, parser): overrides = getattr(parser.values, 'overrides', []) extensions = load_extensions() From cbfafbad1a45aadef2c043c3782489ff6f0b9d62 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:09:16 +0200 Subject: [PATCH 20/29] config: Review nitpicks. --- mopidy/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 258c1cfd..d7d74db5 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -141,7 +141,7 @@ def parse_options(): parser.add_option( b'-o', b'--option', action='append', dest='overrides', type='config_override', - help='`section/key=value` values to override config options.') + help='`section/key=value` values to override config options') return parser.parse_args(args=mopidy_args)[0] @@ -153,7 +153,7 @@ def show_config_callback(option, opt, value, parser): enabled_extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, enabled_extensions) - output = ['# Settings for disabled extensions are not shown.'] + output = ['# Config for disabled extensions are not shown.'] for section_name, schema in config_schemas.items(): options = config.get(section_name, {}) if not options: From 5450905d49b172c38341fc599919a62ab2f97a85 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:12:37 +0200 Subject: [PATCH 21/29] main: Remove dead --debug-thread flag. --- mopidy/__main__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index d7d74db5..71330050 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -134,10 +134,6 @@ def parse_options(): b'--list-deps', action='callback', callback=deps.list_deps_optparse_callback, help='list dependencies and their versions') - parser.add_option( - b'--debug-thread', - action='store_true', dest='debug_thread', - help='run background thread that dumps tracebacks on SIGUSR1') parser.add_option( b'-o', b'--option', action='append', dest='overrides', type='config_override', From e0330e56c1472244704486b2fe494dfc759c71f1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:21:34 +0200 Subject: [PATCH 22/29] config: Get files to load from --config --- mopidy/__main__.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 71330050..b274f768 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -54,14 +54,15 @@ def main(): loop = gobject.MainLoop() options = parse_options() - config_overrides = getattr(options, 'overrides', []) + config_files = options.config.split(':') + config_overrides = options.overrides try: # TODO: we need a two stage logging setup as we want logging for # extension loading and config loading. log.setup_logging(None, options.verbosity_level, options.save_debug_log) extensions = load_extensions() - raw_config = load_config(config_overrides, extensions) + raw_config = load_config(config_files, config_overrides, extensions) extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, extensions) check_old_folders() @@ -134,6 +135,11 @@ def parse_options(): b'--list-deps', action='callback', callback=deps.list_deps_optparse_callback, help='list dependencies and their versions') + parser.add_option( + b'-c', b'--config', + action='store', dest='config', + default='/etc/mopidy/mopidy.conf:$XDG_CONFIG_DIR/mopidy/mopidy.conf', + help='config files to use, colon seperated, later files override') parser.add_option( b'-o', b'--option', action='append', dest='overrides', type='config_override', @@ -142,10 +148,11 @@ def parse_options(): def show_config_callback(option, opt, value, parser): + files = getattr(parser.values, 'config', '').split(':') overrides = getattr(parser.values, 'overrides', []) extensions = load_extensions() - raw_config = load_config(overrides, extensions) + raw_config = load_config(files, overrides, extensions) enabled_extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, enabled_extensions) @@ -235,17 +242,9 @@ def filter_enabled_extensions(raw_config, extensions): return filtered_extensions -def load_config(options, extensions): +def load_config(files, overrides, extensions): parser = configparser.RawConfigParser() - files = [ - '/etc/mopidy/mopidy.conf', - '$XDG_CONFIG_DIR/mopidy/mopidy.conf', - ] - # TODO Add config file given through `options` to `files` - # TODO Replace `files` with single file given through `options` - # TODO expand_path and use xdg when loading. - sources = ['builtin-defaults'] + files + ['command-line'] logging.info('Loading config from: %s', ', '.join(sources)) @@ -273,7 +272,7 @@ def load_config(options, extensions): for section in parser.sections(): raw_config[section] = dict(parser.items(section)) - for section, key, value in options or []: + for section, key, value in overrides or []: raw_config.setdefault(section, {})[key] = value return raw_config From 54a59f5968abbc15415c69fc812cc60ee92118f8 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:24:07 +0200 Subject: [PATCH 23/29] config: Add TODO regarding --show-config behaviour. --- mopidy/__main__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index b274f768..47d9d33c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -148,6 +148,8 @@ def parse_options(): def show_config_callback(option, opt, value, parser): + # TODO: don't use callback for this as --config or -o set after + # --show-config will be ignored. files = getattr(parser.values, 'config', '').split(':') overrides = getattr(parser.values, 'overrides', []) From 0d30db7e5f120994930d9096b1147a0f20f7c12c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:32:36 +0200 Subject: [PATCH 24/29] config: More review fixes and fixed help test. --- mopidy/__main__.py | 7 +++---- mopidy/utils/config.py | 2 +- tests/audio/actor_test.py | 6 +++--- tests/help_test.py | 4 +++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 47d9d33c..280f80e2 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -193,7 +193,7 @@ def check_old_folders(): def load_extensions(): extensions = [] for entry_point in pkg_resources.iter_entry_points('mopidy.ext'): - logger.debug('Loading entrypoint: %s', entry_point) + logger.debug('Loading entry point: %s', entry_point) try: extension_class = entry_point.load() @@ -292,8 +292,7 @@ def validate_config(raw_config, extensions): errors = {} for section_name, schema in sections_and_schemas: if section_name not in raw_config: - logger.error('Config section %s not found', section_name) - process.exit_process() + errors[section_name] = {section_name: 'section not found'} try: items = raw_config[section_name].items() config[section_name] = schema.convert(items) @@ -305,7 +304,7 @@ def validate_config(raw_config, extensions): logger.error('[%s] config errors:', section_name) for key in error: logger.error('%s %s', key, error[key]) - process.exit_process() + sys.exit(1) return config diff --git a/mopidy/utils/config.py b/mopidy/utils/config.py index d2b34e7d..64e0d9ff 100644 --- a/mopidy/utils/config.py +++ b/mopidy/utils/config.py @@ -263,7 +263,7 @@ class ConfigSchema(object): class ExtensionConfigSchema(ConfigSchema): """Sub-classed :class:`ConfigSchema` for use in extensions. - Ensures that `enabled` config value is present. + Ensures that ``enabled`` config value is present. """ def __init__(self): super(ExtensionConfigSchema, self).__init__() diff --git a/tests/audio/actor_test.py b/tests/audio/actor_test.py index 35503472..51786adb 100644 --- a/tests/audio/actor_test.py +++ b/tests/audio/actor_test.py @@ -17,7 +17,7 @@ class AudioTest(unittest.TestCase): settings.MIXER = 'fakemixer track_max_volume=65536' settings.OUTPUT = 'fakesink' self.song_uri = path_to_uri(path_to_data_dir('song1.wav')) - self.audio = audio.Audio.start(None).proxy() + self.audio = audio.Audio.start(config=None).proxy() def tearDown(self): pykka.ActorRegistry.stop_all() @@ -60,7 +60,7 @@ class AudioTest(unittest.TestCase): def test_set_volume_with_mixer_max_below_100(self): settings.MIXER = 'fakemixer track_max_volume=40' - self.audio = audio.Audio.start(None).proxy() + self.audio = audio.Audio.start(config=None).proxy() for value in range(0, 101): self.assertTrue(self.audio.set_volume(value).get()) @@ -81,7 +81,7 @@ class AudioTest(unittest.TestCase): class AudioStateTest(unittest.TestCase): def setUp(self): - self.audio = audio.Audio(None) + self.audio = audio.Audio(config=None) def test_state_starts_as_stopped(self): self.assertEqual(audio.PlaybackState.STOPPED, self.audio.state) diff --git a/tests/help_test.py b/tests/help_test.py index fdef0f52..15c51d2a 100644 --- a/tests/help_test.py +++ b/tests/help_test.py @@ -22,7 +22,9 @@ class HelpTest(unittest.TestCase): self.assertIn('--quiet', output) self.assertIn('--verbose', output) self.assertIn('--save-debug-log', output) - self.assertIn('--list-settings', output) + self.assertIn('--show-config', output) + self.assertIn('--config', output) + self.assertIn('--option', output) def test_help_gst_has_gstreamer_options(self): mopidy_dir = os.path.dirname(mopidy.__file__) From 07fa9548e6925944e9aa7c803f8184ae766b80f5 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:35:07 +0200 Subject: [PATCH 25/29] config: Fix backticks in docstring. --- mopidy/utils/config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/utils/config.py b/mopidy/utils/config.py index 64e0d9ff..30cf873f 100644 --- a/mopidy/utils/config.py +++ b/mopidy/utils/config.py @@ -213,9 +213,9 @@ class ConfigSchema(object): """Logical group of config values that correspond to a config section. Schemas are set up by assigning config keys with config values to - instances. Once setup :meth:`convert` can be called with a list of `(key, - value)` tuples to process. For convienience we also support :meth:`format` - method that can used for printing out the converted values. + instances. Once setup :meth:`convert` can be called with a list of + ``(key, value)`` tuples to process. For convienience we also support + :meth:`format` method that can used for printing out the converted values. """ # TODO: Use collections.OrderedDict once 2.6 support is gone (#344) def __init__(self): From 8087efb319a3703fa5f78947d8ffd541d1a8134a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:57:18 +0200 Subject: [PATCH 26/29] config: Improve printing of disabled extensions in --show-config --- mopidy/__main__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 280f80e2..f2531261 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -158,7 +158,7 @@ def show_config_callback(option, opt, value, parser): enabled_extensions = filter_enabled_extensions(raw_config, extensions) config = validate_config(raw_config, enabled_extensions) - output = ['# Config for disabled extensions are not shown.'] + output = [] for section_name, schema in config_schemas.items(): options = config.get(section_name, {}) if not options: @@ -171,7 +171,9 @@ def show_config_callback(option, opt, value, parser): options = config.get(extension.ext_name, {}) output.append(schema.format(extension.ext_name, options)) else: - output.append('[%s]\nenabled = false' % extension.ext_name) + lines = ['[%s]' % extension.ext_name, 'enabled = false', + '# Config hidden as extension is disabled'] + output.append('\n'.join(lines)) print '\n\n'.join(output) sys.exit(0) From a9445ab251372825db2f70b0d03a894e57c87ea2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:57:54 +0200 Subject: [PATCH 27/29] ext: Log only enabled/disabled extensions at info level --- mopidy/__main__.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index f2531261..c22ee13c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -227,23 +227,28 @@ def load_extensions(): extensions.append(extension) names = (e.ext_name for e in extensions) - logging.info('Found following runnable extensions: %s', ', '.join(names)) + logging.debug('Discovered extensions: %s', ', '.join(names)) return extensions def filter_enabled_extensions(raw_config, extensions): boolean = config_utils.Boolean() - filtered_extensions = [] + enabled_extensions = [] + enabled_names = [] + disabled_names = [] for extension in extensions: # TODO: handle key and value errors. enabled = raw_config[extension.ext_name]['enabled'] if boolean.deserialize(enabled): - filtered_extensions.append(extension) + enabled_extensions.append(extension) + enabled_names.append(extension.ext_name) + else: + disabled_names.append(extension.ext_name) - names = (e.ext_name for e in filtered_extensions) - logging.info('Following extensions will be started: %s', ', '.join(names)) - return filtered_extensions + logging.info('Enabled extensions: %s', ', '.join(enabled_names)) + logging.info('Disabled extensions: %s', ', '.join(disabled_names)) + return enabled_extensions def load_config(files, overrides, extensions): From 5214100854b8e03695434a2c0e8fffb98eb6a138 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:58:13 +0200 Subject: [PATCH 28/29] config: Expand files before printing sources. --- mopidy/__main__.py | 2 +- mopidy/utils/path.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index c22ee13c..fe215636 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -254,6 +254,7 @@ def filter_enabled_extensions(raw_config, extensions): def load_config(files, overrides, extensions): parser = configparser.RawConfigParser() + files = [path.expand_path(f) for f in files] sources = ['builtin-defaults'] + files + ['command-line'] logging.info('Loading config from: %s', ', '.join(sources)) @@ -266,7 +267,6 @@ def load_config(files, overrides, extensions): # Load config from a series of config files for filename in files: - filename = path.expand_path(filename) try: filehandle = codecs.open(filename, encoding='utf-8') parser.readfp(filehandle) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index 7d988a90..4e5a66cd 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -99,6 +99,7 @@ def split_path(path): def expand_path(path): + # TODO: expandvars as well? path = string.Template(path).safe_substitute(XDG_DIRS) path = os.path.expanduser(path) path = os.path.abspath(path) From 8f7e991903dfff467e56ecc6671e53f8d3b5b553 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 2 Apr 2013 23:58:54 +0200 Subject: [PATCH 29/29] config: Do not support -c as allias for --config --- mopidy/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index fe215636..1c0a8515 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -136,7 +136,7 @@ def parse_options(): action='callback', callback=deps.list_deps_optparse_callback, help='list dependencies and their versions') parser.add_option( - b'-c', b'--config', + b'--config', action='store', dest='config', default='/etc/mopidy/mopidy.conf:$XDG_CONFIG_DIR/mopidy/mopidy.conf', help='config files to use, colon seperated, later files override')