GNOME Bugzilla – Bug 700897
Add a Flickr miner
Last modified: 2013-07-07 12:01:44 UTC
Hey, I wrote flickr miner for tracker which uses GdMiner interface from gnome-documents and grilo (so it can be easily extended to some other services). Here's a fork on github: https://github.com/mchalupa/gnome-documents/tree/wip/flickr Regards mch
Great work! It took a quick look at your code. I took the liberty to clean up some elements of your branch and pushed it to a wip/flickr branch on git.gnome.org: https://git.gnome.org/browse/gnome-documents/log/?h=wip/flickr Things that I removed/cleaned up: - gnome-documents should not be using Flickr itself. The only reason we are putting the miner here is to be able to use the base classes. For example, this means that we should not add a FlickrPhoto class in gnome-documents. So, I removed all the JavaScript code from your branch. Those should eventually be going into gnome-photos, or any other application (eg., Background panel) that might want to use the data fetched by the miner. - grilo-0.2.6 has now been released. So I fixed the TODO in configure.ac. - It would be nice to rearrange/split/merge the patches into self-contained logical entities. That way it is easier to review and, more importantly, in future it allows someone to use "git blame / git show" to figure out why something was done the way it was. As a first step, I split out the addition of the DBus service file as a separate commit. - Simplified the copyright notices. You don't need to attribute files that you authored to me. :-) (The Author: is a bit useless anyway, because they quickly get out of date, and "git blame /path/to/file" is much better.) - Removed some stray edits. Things that are left to be done: - Need to investigate why you can't use grl_source_browse_sync instead of grl_source_browse. - Some minor coding style issues. eg., in gd_flickr_miner_class_init. Put a space before the "(" when making a function call, and there shouldn't be a space after the "!" when using it as a logical negation operator. - Remove trailing whitespaces. You can configure git to mark them (with red) when showing the patch.
Created attachment 246687 [details] [review] nao:identifier complies with gnome-photos, some minor fittings Some minor fittings. Also urn prefixes (nao:identifier) has been changed to comply with gnome-photos
Created attachment 246688 [details] [review] Workaround for not-in-set photos Grilo flickr plugin natively don't search for not-in-set photos, so here's a little work around: First search for all photos and then browse the tree and fill in hiearchy.
Created attachment 246689 [details] [review] Mutex, conditional waiting Hash table with currently browsed entries was accessed concurrently from threads, so I added mutex. Consequently I could use GCond to check if hash is empty (instead of while() -> is_empty? -> sleep if not ...)
Created attachment 246690 [details] [review] Prevent from processing the whole entry twice Process entry ends after adjusting parent relationship when the hierarchy is being filled in (instead of updating all data related to the entry)
Hey! I did some work on the miner. The most significant changes: (o) can handle not-in-set photos (o) don't use busy waiting Also I added mutex when accessing hash table (once I got SIGSEGV because of that). Still I don't know why I couldn't use synchronous browsing. With synchronous searching was the same problem. Regards mch
Thanks for the patches. I have now merged this miner in gnome-online-miners, after simplifying things so that mutexes and conditional variables are not required. Great work! PS: we are still waiting for a bugzilla product to be created for gnome-online-miners (bug 702807)