Created attachment 337985 [details] [review]
playlists: add base class for static playlists

Since the all share the same fields internally, it makes
sense to have the static playlists descend from the same
class.

Created attachment 337986 [details] [review]
playlists: remove needless function

Instead of jumping from the single-line callback _on_grilo_read()
to another function, make the code a little bit simpler and merge
these functions into a single one.

Created attachment 337987 [details] [review]
playlists: rewrite static playlist creation routine

This commit makes the creation of static playlists an asynchronous
chain of operations, so we don't block the UI waiting for all the
back and forth of DBus and Tracker.

Review of attachment 337985 [details] [review]:

cleanups always good

::: gnomemusic/playlists.py
@@ +43,1 @@
+class Playlist:

This can be an inner class as well, as long as we don't use it elsewhere.

@@ +45,3 @@
+    ID = None
+    QUERY = None
+    FILTER = None

This one is unused afaics, just drop it.

Review of attachment 337986 [details] [review]:

lgtm

Review of attachment 337987 [details] [review]:

I still see some stutter here. Might be something completely unrelated, but let's see what happens if we make every call async.

The direction of this patch looks like the right way to go.

::: gnomemusic/playlists.py
@@ +148,2 @@
         def callback(obj, result, playlist):
             cursor = obj.query_finish(result)

Outside your scope, but finish should really be try/except.
I think it's important to be valiant on tracker errors to get to a rock solid state.

@@ +150,3 @@
+
+            # Search for the playlist ID
+            while cursor.next(None):

next_async maybe while we are on the async train.

@@ +176,3 @@
+        def playlist_queried_cb(obj, res, playlist):
+            """ Called after the playlist is created and the ID is fetched """
+            cursor = obj.query_finish(res)

try/except

@@ +178,3 @@
+            cursor = obj.query_finish(res)
+
+            if not cursor or not cursor.next():

next_async

@@ +189,3 @@
+        def playlist_created_cb(obj, res, playlist):
+            """ Called when the static playlist is created """
+            data = obj.update_blank_finish(res)

try/except

@@ +191,3 @@
+            data = obj.update_blank_finish(res)
+            playlist_urn = data.get_child_value(0).get_child_value(0).\
+            get_child_value(0).get_child_value(1).get_string()

Can we get clearer here what we are retrieving, it isn't obvious from the code.

I don't like \ for keeping line length in check, but it may be hardly avoidable here. At least use an indent for the 2nd line of the statement.

Created attachment 338561 [details] [review]
playlists: add base class for static playlists

Seems like a leftover from a previous experiment. Removed the FILTER field.

Created attachment 338562 [details] [review]
playlists: rewrite static playlist creation routine

Added try/except guards on various asynchronous functions, and moved the rest of the sync calls to their async variants.

Review of attachment 338561 [details] [review]:

lgtm

Review of attachment 338562 [details] [review]:

lgtm

Thanks for working on this!

This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.