This depends on a single bug. Is it necessary?

(In reply to comment #1)
> This depends on a single bug. Is it necessary?

That'd be a nice feature (considering https://wiki.gnome.org/ThreePointSeven/Features/Owncloud), but not really an important one

Ah! Sorry - the comment in the original report threw me off. This bug is to add support for displaying and playing content hosted in ownCloud instances.

Not sure it needs to depend on bug 708936.

We also depend on ampache support in grilo here (unless this feature gets a priority and we can come up with our own solution)

(In reply to comment #4)
> We also depend on ampache support in grilo here (unless this feature gets a
> priority and we can come up with our own solution)

How come? Is owncloud's music solution really just Ampache?

(In reply to comment #5)
> How come? Is owncloud's music solution really just Ampache?

Looks like so - http://doc.owncloud.org/server/6.0/user_manual/mediaplayer.html, though it might be not the only requirement here

Removing the target-milestone - 3.18 is over.

Removing the available whiteboard - we're not really using it any more.

I've added some general thoughts on behaviour for remote sources in bug 760622. This outlines a fairly advanced set of features that aren't necessary for an initial resolution of this bug.

From a user experience point if view, the main thing I'd be looking for in this bug is that it is possible to browse the music that is stored in the ownCloud instance - the music in ownCloud would be merged into the local database while there's access to it.

Created attachment 332471 [details] [review]
grilo: add owncloud populate methods

Review of attachment 332471 [details] [review]:

I haven't had the opportunity to test this code, this review is just based on looking at the patch.

I see a lot of duplication (the original code is partially to blame). Take the opportunity to clean-up what's there.

Also, new contributions should follow the coding style and have docstrings as noted in the README to be considered for inclusion.

::: gnomemusic/grilo.py
@@ +34,3 @@
 import logging
 import os
+os.environ['GRL_PLUGIN_RANKS'] = 'grl-owncloud-lua:5,grl-local-metadata:4,grl-filesystem:3,grl-tracker-source:2,grl-lastfm-cover:1'

I think local sources should be above the owncloud provider.

@@ +237,3 @@
     @log
+    def populate_owncloud_artists(self, offset, callback, count=-1):
+        # the artist query is effectively getting all albums

If this is true, why not call that function?

@@ +266,3 @@
+            'params': {}
+        }
+        self.populate_owncloud_items(json.dumps(query), offset, callback, count)

populate_owncloud_* is 3 similar function bodies, this can be condensed to generic function. Have them call _populate_owncloud (self, offset, callback, count, endpoint) or something with the generic body.

@@ +319,3 @@
+
+        def _callback(source, param, item, remaining, data, error):
+            callback(source, param, item, remaining, data)

I know this is already happening in the source, but I'm not a big fan of just discarding the error. At least report it.

@@ +330,3 @@
+        else:
+            for k, s in self.owncloud.items():
+                s.query(query, keys, options, _callback, None)

This is partially the same as the tracker populate_items, can't it be reused?

::: gnomemusic/view.py
@@ +762,1 @@
 

I know this is code already here and you just adapted it, but I see 3 the same bits of code in 3 classes.

Created attachment 333942 [details] [review]
grilo: add support for ownCloud source

Search partially depends on support for GrlRelatedKeys to be implemented in lua-factory (grilo) https://bugzilla.gnome.org/show_bug.cgi?id=756203

Created attachment 333952 [details] [review]
grilo: add support for ownCloud source

Rebased against master and resolved conflicts

Review of attachment 333952 [details] [review]:

This needs a rebase with the views/widgets split.

Other than that I really don't like the implementation specific code in the views/widgets. That''s none of your doing, it was already that way, but we need to think of a cleaner way to do this.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-music/issues/9.