[Sugar-devel] [PATCH] Help needed adding microformat support to the activity updater

[Bernie Innocenti]

El Sat, 17-07-2010 a las 10:20 -0700, akashg1611 escribió:

> --- /dev/null
> +++ b/extensions/cpsection/updater/backends/microformat.py
> @@ -0,0 +1,124 @@
> +#!/usr/bin/python
> +# Copyright (C) 2009, Sugar Labs

2010. Also, you might prefer to assign the Copyright to yourself or
Activity Central.


> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> +
> +'''Sugar Microformat Parser.
> +
> +This module implements the microformat parser for the
> +activity updater.
> +'''
> +from HTMLParser import HTMLParser
> +from urllib2 import urlopen, URLError
> +
> +dic = {}

Consider a more descriptive variable name: what is this dictionary for?


> +_fetcher = None
> +
> +
> +class _Microformat_UpdateFetcher(object):
> +
> +    def __init__(self, bundle, completion_cb):
> +
> +        _MicroformatParser('http://wiki.paraguayeduca.org/index.php/Actividades_y_contenidos')

This will block the Sugar shell process until the HTTP request is
completed. Consider an asynchronous approach like the aslo backend does.


> +    def _get_updated_info(self):
> +        try:
> +            version, link = dic[self._bundle.get_bundle_id()]
> +        except KeyError:
> +            print "No bundle with this id"
> +	    return
> +        else:
> +            size = 0
> +            global _fetcher
> +            _fetcher = None
> +            self._completion_cb(self._bundle, version, link, size, None)

Throwing away the fetcher on each invocation is going to cause a new
HTTP request to be issued for each bundle, rather than reusing the
information after the first time.

You might need to rework or extend the current model<->backend interface
to provide a way to tell you when to start a new check. A common OOP
pattern is to employ RAII:

   from backends import microformat
   updater = microformat.new_updater()

   for bundle in bundles:
       updater.fetch_info(bundle, completion_cb)

   # the updater will stay alive until there are references to it
   updater = None

The new_updater() function would return an instance of a
MicroformatUpdater object that will cache the update information after
the first invocation.

However, this improved interface still wouldn't solve the issue of how
to communicate information about bundles that aren't currently
installed.

So, probably, we need to push the loop inside the backend:

   from backends import microformat
   updater = microformat.new_updater()
   updater.fetch_update_info(bundles, completion_cb)


>  from backends import aslo
> -
> +from backends import microformat
>  
>  class UpdateModel(gobject.GObject):
>      __gtype_name__ = 'SugarUpdateModel'
> @@ -76,7 +76,7 @@ class UpdateModel(gobject.GObject):
>          self.emit('progress', UpdateModel.ACTION_CHECKING, bundle.get_name(),
>                    current, total)
>  
> -        aslo.fetch_update_info(bundle, self.__check_completed_cb)
> +        microformat.fetch_info(bundle, self.__check_completed_cb)

Either we make this configurable, or we stop importing the aslo backend.

Once the ASLO microformat support is completed, I'd be in favor of
killing the aslo backend altogether. What do the Sugar maintainers
think?

-- 
   // Bernie Innocenti - http://codewiz.org/
 \X/  Sugar Labs       - http://sugarlabs.org/