Repository: thesquelched/suggestive

Issue:
crash on startup
When starting up suggestive it crashes with a KeyError.

```
carnager@caprica ~/suggestive/suggestive > suggestive             
Traceback (most recent call last):
  File "/usr/bin/suggestive", line 9, in <module>
    load_entry_point('suggestive==0.4.0', 'console_scripts', 'suggestive')()
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/app.py", line 586, in main
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/app.py", line 566, in run
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/app.py", line 223, in __init__
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/app.py", line 53, in __init__
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/app.py", line 97, in initialize_buffers
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/app.py", line 114, in create_playlist_buffer
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/playlist.py", line 471, in __init__
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/playlist.py", line 90, in __init__
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/playlist.py", line 220, in update_model
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/playlist.py", line 191, in track_models
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/playlist.py", line 165, in playlist_tracks
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/mstat.py", line 140, in database_tracks_from_mpd
  File "/usr/lib/python3.5/site-packages/suggestive-0.4.0-py3.5.egg/suggestive/mstat.py", line 140, in <listcomp>
KeyError: 'flac/Motorpsycho/1993 Demon Box/CD 1/01-Waiting for the One.flac'
```

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 414275e..949e00f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,11 @@
+Version 0.4.1
+=============
+
+Bug Fixes
+---------
+- Add mpd connection retries to MPD loader
+- Fix error when log directory doesn't exist
+
 Version 0.4.0
 =============
 
diff --git a/README.md b/README.md
index c5c8d15..8bbf90d 100644
--- a/README.md
+++ b/README.md
@@ -57,6 +57,8 @@ Here is a minimal `suggestive` configuration file:
 [lastfm]
 user = my_lastfm_user
 api_key = 0123456789abcdefghijklmnopqrstuv
+
+# For LastFM write access (optional)
 api_secret = 141iojhu789uihy78uiho9uih89080
 ```
 
diff --git a/suggestive/_version.py b/suggestive/_version.py
index 0ed5398..90134aa 100644
--- a/suggestive/_version.py
+++ b/suggestive/_version.py
@@ -1,2 +1,2 @@
-__version_info__ = (0, 4, 0)
+__version_info__ = (0, 4, 1)
 __version__ = '.'.join(str(piece) for piece in __version_info__)
diff --git a/suggestive/app.py b/suggestive/app.py
index d751041..1de628d 100644
--- a/suggestive/app.py
+++ b/suggestive/app.py
@@ -25,8 +25,7 @@ import urwid
 import logging
 from logging.handlers import RotatingFileHandler
 import threading
-import os.path
-from os import remove
+import os
 import sys
 import gzip
 
@@ -509,7 +508,12 @@ def initialize_logging(conf):
                 for line in sf:
                     df.write(line)
 
-        remove(source)
+        os.remove(source)
+
+    try:
+        os.makedirs(os.path.dirname(conf.log_file()))
+    except IOError:
+        pass
 
     handler = RotatingFileHandler(
         conf.log_file(),
diff --git a/suggestive/mstat.py b/suggestive/mstat.py
index d6d22a7..61ff8d8 100644
--- a/suggestive/mstat.py
+++ b/suggestive/mstat.py
@@ -125,19 +125,22 @@ def database_tracks_from_mpd(conf, tracks_info):
     """
     Return the database Track object corresponding to track info from MPD
     """
+    track_filenames = [info['file'] for info in tracks_info]
+
     with session_scope(conf, commit=False) as session:
-        filenames = [info['file'] for info in tracks_info]
-        db_tracks = session.query(Track).\
-            options(
-                subqueryload(Track.album),
-                subqueryload(Track.artist),
-                subqueryload(Track.lastfm_info)
-            ).\
-            filter(Track.filename.in_(filenames)).\
-            all()
+        tracks_by_filename = {}
 
-        tracks_by_filename = {t.filename: t for t in db_tracks}
-        return [tracks_by_filename[info['file']] for info in tracks_info]
+        for chunk in partition(track_filenames, 128):
+            db_tracks = (session.query(Track).
+                         options(subqueryload(Track.album),
+                                 subqueryload(Track.artist),
+                                 subqueryload(Track.lastfm_info)).
+                         filter(Track.filename.in_(chunk)).
+                         all())
+
+            tracks_by_filename.update({t.filename: t for t in db_tracks})
+
+        return [tracks_by_filename[filename] for filename in track_filenames]
 
 
 def get_scrobbles(conf, limit, offset=None):
@@ -319,11 +322,11 @@ class ScrobbleLoader(object):
         if not len(scrobbles):
             return 0
 
-        first = next(scrobbles)
+        first = scrobbles[0]
         self.load_scrobble(session, first)
 
         track = None  # Necessary if there was only one scrobble total
-        for track in scrobbles:
+        for track in scrobbles[1:]:
             self.load_scrobble(session, track)
 
         last = track or first
@@ -339,8 +342,17 @@ class MpdLoader(object):
     Synchronizes the MPD and suggestive databases
     """
 
-    def __init__(self, mpd):
-        self.mpd = mpd
+    def __init__(self, conf):
+        self._conf = conf
+        self._mpd = initialize_mpd(conf)
+
+    @property
+    def mpd(self):
+        return self._mpd
+
+    @property
+    def conf(self):
+        return self._conf
 
     def load_track(self, session, db_artist, db_album, info):
         """
@@ -442,6 +454,7 @@ class MpdLoader(object):
 
         logger.debug('Deleted {} empty albums'.format(len(empty)))
 
+    @mpd_retry
     def check_duplicates(self, session):
         """
         Check for albums with duplicate tracks
@@ -492,11 +505,19 @@ class MpdLoader(object):
 
         return by_artist_album
 
+    @mpd_retry
+    def _list_mpd_files(self):
+        return self.mpd.list('file')
+
+    @mpd_retry
+    def _mpd_info(self, path):
+        return self.mpd.listallinfo(path)
+
     def load(self, session):
         """
         Synchronize MPD and suggestive databases
         """
-        files_in_mpd = set(self.mpd.list('file'))
+        files_in_mpd = set(self._list_mpd_files())
         files_in_db = set(item.filename for item in session.query(
             Track.filename).all())
 
@@ -509,8 +530,7 @@ class MpdLoader(object):
             logger.debug('Missing files:\n  {}'.format(
                 '\n  '.join(missing)))
             missing_info = list(
-                chain.from_iterable(self.mpd.listallinfo(path)
-                                    for path in missing))
+                chain.from_iterable(self._mpd_info(path) for path in missing))
 
             by_artist_album = self.segregate_track_info(missing_info)
             self.load_by_artist_album(session, by_artist_album)
@@ -750,8 +770,7 @@ def update_mpd(config):
         albums_start = session.query(Album).count()
         tracks_start = session.query(Track).count()
 
-        mpdclient = initialize_mpd(config)
-        mpd_loader = MpdLoader(mpdclient)
+        mpd_loader = MpdLoader(config)
         mpd_loader.load(session)
 
         session.commit()
diff --git a/tox.ini b/tox.ini
index d5c3ff3..410c552 100644
--- a/tox.ini
+++ b/tox.ini
@@ -5,10 +5,12 @@ envlist = py33,py34,py35
 deps = -r{toxinidir}/test-requirements.txt
 
 commands = py.test -m "not live" --ignore=build --ignore=suggestive/alembic \
-           --pep8 --flakes --cov={envsitepackagesdir}/suggestive -rs -v {posargs}
+           --ignore=venv --pep8 --flakes \
+           --cov={envsitepackagesdir}/suggestive -rs -v {posargs}
 
 [testenv:coverage]
 deps = -r{toxinidir}/test-requirements.txt
 
 commands = py.test -m "not live" --ignore=build --ignore=suggestive/alembic \
-           --cov={envsitepackagesdir}/suggestive --cov-report=html
+           --ignore=venv --cov={envsitepackagesdir}/suggestive \
+           --cov-report=html


Test patch:
diff --git a/tests/test_mstat.py b/tests/test_mstat.py
new file mode 100644
index 0000000..77a90ea
--- /dev/null
+++ b/tests/test_mstat.py
@@ -0,0 +1,45 @@
+from unittest.mock import patch, MagicMock
+from suggestive import mstat
+
+
+class TestMpdLoader:
+
+    @patch('suggestive.mstat.initialize_mpd')
+    def test_check_duplicates(self, init_mpd):
+        init_mpd.side_effect = [
+            MagicMock(find=MagicMock(side_effect=OSError)),
+            MagicMock(),
+        ]
+
+        session = MagicMock()
+        (session.query.return_value.join.return_value.group_by.return_value
+         .having.return_value.all.return_value) = [MagicMock()]
+
+        loader = mstat.MpdLoader(None)
+        loader.check_duplicates(session)
+
+        assert init_mpd.call_count == 2
+
+    @patch('suggestive.mstat.initialize_mpd')
+    def test_list_mpd_files(self, init_mpd):
+        init_mpd.side_effect = [
+            MagicMock(list=MagicMock(side_effect=OSError)),
+            MagicMock(),
+        ]
+
+        loader = mstat.MpdLoader(None)
+        loader._list_mpd_files()
+
+        assert init_mpd.call_count == 2
+
+    @patch('suggestive.mstat.initialize_mpd')
+    def test_mpd_info(self, init_mpd):
+        init_mpd.side_effect = [
+            MagicMock(listallinfo=MagicMock(side_effect=OSError)),
+            MagicMock(),
+        ]
+
+        loader = mstat.MpdLoader(None)
+        loader._mpd_info(None)
+
+        assert init_mpd.call_count == 2