[Dextrose] Dextrose Digest, Vol 15, Issue 11

Ajay Garg ajaygargnsit at gmail.com
Tue Sep 20 17:57:42 EDT 2011


Hi all.
Thanks for the feedback. Looking for more!!

Particularly, waiting to hear regarding the cron-script approach (see
comments below)
PLUS
waiting to hear for an ideal solution, particularly from Sascha and Aleksey.





On Tue, Sep 20, 2011 at 8:52 PM, <dextrose-request at lists.sugarlabs.org>wrote:

> Send Dextrose mailing list submissions to
>        dextrose at lists.sugarlabs.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
>        http://lists.sugarlabs.org/listinfo/dextrose
> or, via email, send a message with subject or body 'help' to
>        dextrose-request at lists.sugarlabs.org
>
> You can reach the person managing the list at
>        dextrose-owner at lists.sugarlabs.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Dextrose digest..."
>
>
> Today's Topics:
>
>   1. Re: [PATCH] Resolution for ticket au#887. (Anish Mangal)
>   2. Re: [PATCH] Resolution for ticket au#887. (Frederick Grose)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue, 20 Sep 2011 18:11:45 +0530
> From: Anish Mangal <anish at activitycentral.org>
> To: Ajay Garg <ajaygargnsit at gmail.com>
> Cc: Ajay Garg <ajay at sugarlabs.org>, dextrose at lists.sugarlabs.org
> Subject: Re: [Dextrose] [PATCH] Resolution for ticket au#887.
> Message-ID: <4E788A09.8020107 at activitycentral.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Thanks for the patch!
>
> On 09/20/2011 05:22 PM, Ajay Garg wrote:
> > === Usability notes ===
> >
> > 1. Now, besides an interactive 'Software Update', a background
> >    thread also checks for available activity updates. To prevent
> >    possible collisions in a rare situation when the interactive
> >    session and the background thread might run at the same time,
> >    synchronization has been used.
> >
>
> Would a simple lock file mechanism work?
>

Condition variable has been used as the synchronization primitive, because
of the following reasons ::

1. It allows holding the underlying lock for two small intervals - one for
testing-and-unsetting the condition (during acquire()), and another for
setting the
   condition (during release().
   This is in contrast to a lock mechanism, wherein the lock is held for the
entire duration of mutually-exlusive-desired operation.

2. More importantly, there is no "ownership" of the underlying lock (except
for the two small intervals described above).
   This becomes all the more important, since there are a whole lot of
threads involved. For eg, *self._model.check_updates()* begins in one
thread,
   while the *_finished_checking()* callback is called in the context of
another thread.
   Herein, the so called critical section is entered via the first thread
(by testing-and-unsetting the condition), and the critical section released
in the
   second thread (by setting the condition).
   Had there been a lock used, this would have resulted in undefined
behaviour, since only the thread holding the lock may release it.








> > 2. When a user enters into 'Software Update' section-view,
> >    exclusive control is gained. The exclusive control is
> >    retained unless and until this section-view is exited
> >    (via 'Cancel' or 'Ok' toolbar buttons).
> >
> >    If the background thread wishes to run, it waits,
> >    while the user interacts with the section-view.
> >
> > 3. If the background thread is running, it has exclusive control.
> >    If the user wishes to enter into an interactive session (by
> >    clicking onto the 'Software Update' section-view-icon),
> >    following happens :
> >
> >    a. The section-view shows 'Getting ready...' markup,
> >       with the message 'Waiting for activity-update-
> >       notifier thread to relinquish control...'
> >    b. The (interactive) thread waits, until the background
> >       thread relinquishes control.
> >    c. After the (exclusive) control is retained, the
> >       interactive thread proceeds normally, checking
> >       for updates, followed by user-wish of installing
> >       updates.
> >
> > === Implementation notes ===
> >
> > For this, comments have been added into the code appropriately.
> >
> > === Configuration notes ===
> >
> > 1. Following gconf keys can be used to configure this feature.
> >
> >    a. KEY     :  /desktop/sugar/activity_notifier_enabled
> >       PURPOSE :  Enable/Disable the notifier background thread
> >       TYPE    :  boolean
> >       DEFAULT :  true (enables the notifier background thread)
> >
> >    b. KEY     :  /desktop/sugar/activity_notifier_interval_first_time
> >       PURPOSE :  Interval after which the background thread runs for
> >                  the first time
> >       TYPE    :  int
> >       DEFAULT :  120 (seconds)
> >
> >    c. KEY     :
>  /desktop/sugar/activity_notifier_interval_second_time_onwards
> >       PURPOSE :  Interval for the subsequen runs
> >       TYPE    : int
> >       DEFAULT : 604800 (seconds). This is equivalent to 7 days.
> >
> > 2. Note that these keys would be used on a pre-configured basis.
> >    No change-notifier callbacks have been added into the code,
> >    to detect key-changes during runtime.
> >
> > === Design enhancements/changes ===
> >
> > 1. A mechanism for loading "daemon" classes has been added into
> >    "src/jarabe/controlpanel/homewindow.py" file.
> >    It searches for an attribute "DAEMON" in the "__init__.py" files
> >    of "extensions/cpsection" modules. If the attribute is found, the
> >    corresponding class is loaded.
> >
> >    Currently, only "Software Update" module has the attribute "DAEMON"
> >    in "__init__.py". The corresponding class - "ActivityNotifier" is
> loaded.
> >    Being a subclass of "threading.Thread", this class runs as a daemon.
> >
> >    Note that the loading of these "daemon" classes occurs during the
> loading
> >    of home-window, since that is the first thing that happens after boot.
> >
> > Signed-off-by: Ajay Garg <ajay at sugarlabs.org>
> > ---
> >  data/sugar.schemas.in                    |   36 +++++++++
> >  extensions/cpsection/updater/Makefile.am |    3 +-
> >  extensions/cpsection/updater/__init__.py |    1 +
> >  extensions/cpsection/updater/daemon.py   |   93 +++++++++++++++++++++++
> >  extensions/cpsection/updater/model.py    |   24 ++++++
> >  extensions/cpsection/updater/view.py     |  118
> +++++++++++++++++++++++------
> >  src/jarabe/controlpanel/gui.py           |   20 +++++
> >  src/jarabe/controlpanel/sectionview.py   |   32 ++++++++
> >  src/jarabe/desktop/homewindow.py         |   35 +++++++++
> >  9 files changed, 336 insertions(+), 26 deletions(-)
> >  create mode 100644 extensions/cpsection/updater/daemon.py
> >
> > diff --git a/data/sugar.schemas.in b/data/sugar.schemas.in
> > index aa031da..54ba4e2 100644
> > --- a/data/sugar.schemas.in
> > +++ b/data/sugar.schemas.in
> > @@ -129,6 +129,42 @@
> >      </schema>
> >
> >      <schema>
> > +      <key>/schemas/desktop/sugar/activity_notifier_enabled</key>
> > +      <applyto>/desktop/sugar/activity_notifier_enabled</applyto>
> > +      <owner>sugar</owner>
> > +      <type>bool</type>
> > +      <default>true</default>
> > +      <locale name="C">
> > +        <short>Flag to turn on/off the activity-update-notifier.</short>
> > +        <long>This key contains the flag, which can be used to turn
> on/off the activity-update-notifier.</long>
> > +      </locale>
> > +    </schema>
> > +
> > +    <schema>
> > +
>  <key>/schemas/desktop/sugar/activity_notifier_interval_first_time</key>
> > +
>  <applyto>/desktop/sugar/activity_notifier_interval_first_time</applyto>
> > +      <owner>sugar</owner>
> > +      <type>int</type>
> > +      <default>120</default>
> > +      <locale name="C">
> > +        <short>Interval for activity-update-notifier timer (in seconds),
> for first run</short>
> > +        <long>This key contains the interval, after which, the
> activity-update-notifier runs for the first time. Default value is 120
> seconds.</long>
> > +      </locale>
> > +    </schema>
> > +
> > +    <schema>
> > +
>  <key>/schemas/desktop/sugar/activity_notifier_interval_second_time_onwards</key>
> > +
>  <applyto>/desktop/sugar/activity_notifier_interval_second_time_onwards</applyto>
> > +      <owner>sugar</owner>
> > +      <type>int</type>
> > +      <default>604800</default>
> > +      <locale name="C">
> > +        <short>Interval for activity-update-notifier timer (in seconds),
> for second run onwards</short>
> > +        <long>This key contains the interval, for
> activity-update-notifier, for second and subsequent runs. Default value is
> 604800 seconds (~ 7 days)</long>
> > +      </locale>
> > +    </schema>
> > +
>
> Looking at the problem overall, we just need one gconf key to enable,
> disable the notifications. The other two keys are not really needed.
> Perhaps we could just follow the approach dextrose-updater uses. It
> checks every hour if it has run for the day, and if it has, does nothing.
>
> cron jobs are ideal for periodic tasks.
>

Just wonder, if the cron jobs are suitable for doing jobs other than pure
backend work (herein, UI notification needs to be added).
I need some assistance, as to how to handle UI-python code in cron jobs.








>
> > +    <schema>
> >        <key>/schemas/desktop/sugar/backup_url</key>
> >        <applyto>/desktop/sugar/backup_url</applyto>
> >        <owner>sugar</owner>
> > diff --git a/extensions/cpsection/updater/Makefile.am
> b/extensions/cpsection/updater/Makefile.am
> > index 897dbf3..d346cac 100644
> > --- a/extensions/cpsection/updater/Makefile.am
> > +++ b/extensions/cpsection/updater/Makefile.am
> > @@ -5,4 +5,5 @@ sugardir = $(pkgdatadir)/extensions/cpsection/updater
> >  sugar_PYTHON =               \
> >       __init__.py     \
> >       model.py        \
> > -     view.py
> > +     view.py         \
> > +     daemon.py
> > diff --git a/extensions/cpsection/updater/__init__.py
> b/extensions/cpsection/updater/__init__.py
> > index 6010615..7d4b432 100644
> > --- a/extensions/cpsection/updater/__init__.py
> > +++ b/extensions/cpsection/updater/__init__.py
> > @@ -17,6 +17,7 @@
> >  from gettext import gettext as _
> >
> >  CLASS = 'ActivityUpdater'
> > +DAEMON = 'ActivityNotifier'
> >  ICON = 'module-updater'
> >  TITLE = _('Software update')
> >  KEYWORDS = ['software', 'activity', 'update']
> > diff --git a/extensions/cpsection/updater/daemon.py
> b/extensions/cpsection/updater/daemon.py
> > new file mode 100644
> > index 0000000..a1b54e1
> > --- /dev/null
> > +++ b/extensions/cpsection/updater/daemon.py
> > @@ -0,0 +1,93 @@
> > +# Copyright (C) 2011, Ajay Garg
> > +#
>
> Include your email address (preferable)
>

Done.








>
> > +# 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, see <http://www.gnu.org/licenses/>.
> > +
> > +import logging
> > +import threading
> > +import time
> > +
> > +from gettext import gettext as _
> > +
> > +import gconf
> > +import gobject
> > +
> > +from jarabe import frame
> > +from model import UpdateModel
> > +from model import ModelAttributesHolder
> > +
> > +gobject.threads_init()
> > +
> > +client = gconf.client_get_default()
> > +NOTIFIER_ENABLED = \
> > +        client.get_bool('/desktop/sugar/activity_notifier_enabled')
> > +NOTIFIER_INTERVAL_FIRST_TIME = \
> > +
>  client.get_int('/desktop/sugar/activity_notifier_interval_first_time')
> > +NOTIFIER_INTERVAL_SECOND_TIME_ONWARDS = \
> > +
>  client.get_int('/desktop/sugar/activity_notifier_interval_second_time_onwards')
> > +
> > +
> > +class ActivityNotifier(threading.Thread):
> > +
> > +
> > +    def __init__(self):
> > +        threading.Thread.__init__(self)
> > +        self.start()
> > +
> > +
> > +    def run(self):
> > +        if NOTIFIER_ENABLED:
> > +            self._model = UpdateModel()
> > +            self._model.connect('progress', self._progress_cb)
> > +
> > +            time.sleep(NOTIFIER_INTERVAL_FIRST_TIME)
> > +
> > +            while 1:
> > +                self._timer_cb()
> > +                time.sleep(NOTIFIER_INTERVAL_SECOND_TIME_ONWARDS)
> > +
>
> Though I'm opposed to this approach in general, here we should use
> gobject.timeout_add()
>
> time.sleep() won't work well with gtk threading IMO
>

*The most interesting ............*

I tried converting the time.sleep calls to gobject.timeout_add(), and hoped
that there wouldn't be a problem.
But if the Software-Update section view is opened, and the background thread
tries to check the condition variable, UI freezes :-( ...

After a good two hours of debugging, I found out the root cause ::
                      Normally, there is one main thread, which is running.
Now, when the section view is opened, this main
                      thread acquires the exclusive control. Now, when the
background thread runs using gobject.timeout_add(),
                      *it runs in the context of the same main thread*. In
fact, whenever any operation is delegated via gobject.idle_add(),
                       or gobject.timeout_add(), it is the context of the
*same main thread* in which the operation runs.

So, in the present case, the section-view UI was being run in the context of
the main thread, and this main thread had the exclusive control. Now, during
its idle time, it tried to run the background update operation. Because
mutual exclusion is needed, it tried to gain exclusive control for the
background thread, via testing the condition variable. Since condition
variable is unlike a recursive lock, the thread could not **re-gain**
exclusive control. As a result, it called the wait() function, and went to
sleep - which was ok from the background operation perspective, but going to
sleep also freezed the UI, and hence the application!!!!

if the background operation is not called via gobject.add_timeout(), it runs
in the context of background thread itself, and the wait() causes just this
background thread to wait, while keeping the UI-main-thread responsive.
Obviously, as per the design, when the section-view is exited, the
UI-main-thread notifies the waiting background thread, and the background
thread proceeds for its operation normally.

Surely was a tremendous experience for me :)








> > +
> > +    def _timer_cb(self):
> > +        self.timer_cb_execute()
> > +        return True
> > +
>
> the '_cb' is meant to signify that its a callback function, which it
> isn't. Also, minor point, _cb (callback) functions in general should
> start with two underscores, so __timer_cb()
>

Done.
Removed the underscores, and also renamed the functions (specifically
removing the '_cb' part)







> > +
> > +    def timer_cb_execute(self):
> > +        ModelAttributesHolder.grab_exclusive_control(True)
> > +
> > +        self._model.check_updates()
> > +        return True
> > +
> > +
> > +    def _progress_cb(self, model, action, bundle_name, current, total):
> > +        if current == total and action == UpdateModel.ACTION_CHECKING:
> > +            self._finished_checking()
> > +
> > +
> > +    def _finished_checking(self):
> > +        available_updates = len(self._model.updates)
> > +
> > +        ModelAttributesHolder.yield_exclusive_control()
> > +
> > +        if available_updates:
> > +            title = _('Activity Updates available')
> > +            msg = _('%d updates available.\n'
> > +                    'Please run \'Software Update\' '
> > +                    'to install these updates.' % available_updates)
> > +            gobject.idle_add(lambda:
> > +                    frame.get_view().add_message(summary=title,
> > +                                                 body=msg))
> > +        else:
> > +            # Do nothing. No updates available.
> > +            logging.debug('No activity-updates found. Not showing any'
> > +                          'notification')
> > diff --git a/extensions/cpsection/updater/model.py
> b/extensions/cpsection/updater/model.py
> > index 39d0b1f..5e6fc73 100755
> > --- a/extensions/cpsection/updater/model.py
> > +++ b/extensions/cpsection/updater/model.py
> > @@ -38,6 +38,30 @@ from sugar.bundle.bundleversion import
> NormalizedVersion
> >  from jarabe.model import bundleregistry
> >
> >  from backends import microformat
> > +from threading import Condition
> > +
> > +
> > +class ModelAttributesHolder():
> > +
> > +    background_action = {}
> > +    flag = True
> > +    condition = Condition()
> > +
> > +    @staticmethod
> > +    def grab_exclusive_control(is_bg_action):
> > +        ModelAttributesHolder.condition.acquire()
> > +        while not ModelAttributesHolder.flag:
> > +            ModelAttributesHolder.condition.wait()
> > +        ModelAttributesHolder.flag = False
> > +        ModelAttributesHolder.background_action = is_bg_action
> > +        ModelAttributesHolder.condition.release()
> > +
> > +    @staticmethod
> > +    def yield_exclusive_control():
> > +        ModelAttributesHolder.condition.acquire()
> > +        ModelAttributesHolder.flag = True
> > +        ModelAttributesHolder.condition.notify()
> > +        ModelAttributesHolder.condition.release()
> >
> >  class MetaBundle():
> >
> > diff --git a/extensions/cpsection/updater/view.py
> b/extensions/cpsection/updater/view.py
> > index d257b56..a67b894 100644
> > --- a/extensions/cpsection/updater/view.py
> > +++ b/extensions/cpsection/updater/view.py
> > @@ -29,12 +29,62 @@ from sugar.graphics.icon import Icon,
> CellRendererIcon
> >  from jarabe.controlpanel.sectionview import SectionView
> >
> >  from model import UpdateModel
> > +from model import ModelAttributesHolder
> >
> >  _DEBUG_VIEW_ALL = True
> >
> >
> >  class ActivityUpdater(SectionView):
> >
> > +    """
> > +    Notes:
> > +
> > +     1. This class, represents the 'Software Update' section-view.
> > +        A new instance of ActivityUpdater is created, everytime user
> > +        enters into the 'Software Update' section-view.
> > +
> > +     2. Now, besides an interactive 'Software Update', a background
> > +        thread also checks for available activity updates. (See
> > +        'daemon.py'). To prevent possible collisions in a rare
> > +        situation when the interactive session and the background
> > +        thread might run at the same time, synchronization has
> > +        been used.
> > +        (
> > +         If both are allowed to run simultaneously, I was getting
> > +         errors in gio.File.read_async()
> > +        )
> > +
> > +     3. When a user enters into 'Software Update' section-view,
> > +        exclusive control is gained. The exclusive control is
> > +        retained unless and until this section-view is exited
> > +        (via 'Cancel' or 'Ok' toolbar buttons).
> > +
> > +        See overridden 'self.post_show_section_view()' and
> > +        'self.post_exit()' methods.
> > +
> > +        If the background thread wishes to run, it waits,
> > +        while the user interacts with the section-view.
> > +
> > +
> > +     4. If the background thread is running, it has exclusive control.
> > +        If the user wishes to enter into an interactive session (by
> > +        clicking onto the 'Software Update' section-view-icon),
> > +        following happens :
> > +
> > +            a. The section-view shows 'Getting ready...' markup,
> > +               with the message 'Waiting for activity-update-
> > +               notifier thread to relinquish control...'
> > +            b. The (interactive) thread waits, until the background
> > +               thread relinquishes control.
> > +            c. After the (exclusive) control is retained, the
> > +               interactive thread proceeds normally, checking
> > +               for updates, followed by user-wish of installing
> > +               updates.
> > +
> > +        See overridden 'self.post_show_section_view()' method.
> > +    """
> > +
> > +
> >      def __init__(self, model, alerts):
> >          SectionView.__init__(self)
> >
> > @@ -68,7 +118,13 @@ class ActivityUpdater(SectionView):
> >          self._update_box = None
> >          self._progress_pane = None
> >
> > -        self._refresh()
> > +        # Set up an inital set-up for section-view.
> > +        top_message = _('Getting ready...')
> > +        self._top_label.set_markup('<big>%s</big>' % top_message)
> > +        self.__progress_cb('progress', UpdateModel.ACTION_CHECKING,
> > +                           'Waiting for '
> > +                           'activity-update-notifier thread to '
> > +                           'relinquish control', 0, 3, False)
> >
> >      def _switch_to_update_box(self):
> >          if self._update_box in self.get_children():
> > @@ -113,7 +169,8 @@ class ActivityUpdater(SectionView):
> >              self.remove(self._update_box)
> >              self._update_box = None
> >
> > -    def __progress_cb(self, model, action, bundle_name, current, total):
> > +    def __progress_cb(self, model, action, bundle_name, current, total,
> > +                      check_for_background_action=True):
>
> /me doesn't like the approach of checking this so deep in the code.
>
> >          if current == total and action == UpdateModel.ACTION_CHECKING:
> >              self._finished_checking()
> >              return
> > @@ -121,36 +178,41 @@ class ActivityUpdater(SectionView):
> >              self._finished_updating(int(current))
> >              return
> >
> > -        if action == UpdateModel.ACTION_CHECKING:
> > -            message = _('%s...') % bundle_name
> > -        elif action == UpdateModel.ACTION_DOWNLOADING:
> > -            message = _('Downloading %s...') % bundle_name
> > -        elif action == UpdateModel.ACTION_UPDATING:
> > -            message = _('Updating %s...') % bundle_name
> > +        if (not check_for_background_action) or \
> > +           ((check_for_background_action) and \
>
> Same as above
>
> > +             (not ModelAttributesHolder.background_action)):
> > +               if action == UpdateModel.ACTION_CHECKING:
> > +                   message = _('%s...') % bundle_name
> > +               elif action == UpdateModel.ACTION_DOWNLOADING:
> > +                   message = _('Downloading %s...') % bundle_name
> > +               elif action == UpdateModel.ACTION_UPDATING:
> > +                   message = _('Updating %s...') % bundle_name
> >
> > -        self._switch_to_progress_pane()
> > -        self._progress_pane.set_message(message)
> > -        self._progress_pane.set_progress(current / float(total))
> > +               self._switch_to_progress_pane()
> > +               self._progress_pane.set_message(message)
> > +               self._progress_pane.set_progress(current / float(total))
> >
> >      def _finished_checking(self):
> >          logging.debug('ActivityUpdater._finished_checking')
> >          available_updates = len(self._model.updates)
> > -        if not available_updates:
> > -            top_message = _('Your software is up-to-date')
> > -        else:
> > -            top_message = ngettext('You can install %s update',
> > -                                   'You can install %s updates',
> > -                                   available_updates)
> > -            top_message = top_message % available_updates
> > -            top_message = gobject.markup_escape_text(top_message)
> >
> > -        self._top_label.set_markup('<big>%s</big>' % top_message)
> > +        if not ModelAttributesHolder.background_action:
>
> Same as above
>

Done.

In fact, the intent was right, as there are separate "__progress_cb"  and
"_finished_updating" methods for the section-view, and the background
thread.
The application was running fine, calling the corresponding callbacks.
The "ModelAttributesHolder.background_action" turns out to be redundant, as
each type (section-view and the background thread) run their own methods.
Cleaned up by removing the redundant checks.

My mistake :-(









> > +            if not available_updates:
> > +                top_message = _('Your software is up-to-date')
> > +            else:
> > +                top_message = ngettext('You can install %s update',
> > +                                       'You can install %s updates',
> > +                                       available_updates)
> > +                top_message = top_message % available_updates
> > +                top_message = gobject.markup_escape_text(top_message)
> > +
> > +            self._top_label.set_markup('<big>%s</big>' % top_message)
> >
> > -        if not available_updates:
> > -            self._clear_center()
> > -        else:
> > -            self._switch_to_update_box()
> > -            self._update_box.refresh()
> > +            if not available_updates:
> > +                self._clear_center()
> > +            else:
> > +                self._switch_to_update_box()
> > +                self._update_box.refresh()
> >
> >      def __refresh_button_clicked_cb(self, button):
> >          self._refresh()
> > @@ -180,6 +242,12 @@ class ActivityUpdater(SectionView):
> >      def undo(self):
> >          self._model.cancel()
> >
> > +    def post_show_section_view(self):
> > +        ModelAttributesHolder.grab_exclusive_control(False)
> > +        self._refresh()
> > +
> > +    def post_exit(self):
> > +        ModelAttributesHolder.yield_exclusive_control()
> >
> >  class ProgressPane(gtk.VBox):
> >      """Container which replaces the `ActivityPane` during refresh or
>
> - -1 to modifying big parts of code where we could check at just one point
> and do something/nothing accordingly.
>

Removed redundant checks, and as explained in the previous comment, used OOP
concepts to call the corresponding callbacks per type.









>
> > diff --git a/src/jarabe/controlpanel/gui.py
> b/src/jarabe/controlpanel/gui.py
> > index b4879c6..f420a96 100644
> > --- a/src/jarabe/controlpanel/gui.py
> > +++ b/src/jarabe/controlpanel/gui.py
> > @@ -177,6 +177,18 @@ class ControlPanel(gtk.Window):
> >              self._options[option]['button'] = sectionicon
> >
> >      def _show_main_view(self):
> > +
> > +        # 1. This method has two callers.
> > +        # 2. One, when the main_view is shown upon clicking on
> > +        #    'My Settings' from the home-window buddy-menu.
> > +        # 3. Second, when the main_view is shown, after exiting
> > +        #    from a section_view.
> > +        # 4. In the second case, when we are coming out of the
> > +        #    section-view, call the 'post_exit' method for the
> section-view
> > +        if self._section_view:
> > +            self._section_view.post_exit()
> > +
> > +
> >          self._set_toolbar(self._main_toolbar)
> >          self._main_toolbar.show()
> >          self._set_canvas(self._scrolledwindow)
> > @@ -244,6 +256,14 @@ class ControlPanel(gtk.Window):
> >          self._main_view.modify_bg(gtk.STATE_NORMAL,
> >                                    style.COLOR_BG_CP.get_gdk_color())
> >
> > +
> > +        # 1. Adding 'post_show_section_view()' method.
> > +        # 2. Please see 'jarabe.controlpanel.sectionview.SectionView'
> > +        #    for definition.
> > +        # 3. This needs to done in 'idle' mode, to prevent
> > +        #    UI freeze.
> > +        gobject.idle_add(self._section_view.post_show_section_view)
> > +
> >      def set_section_view_auto_close(self):
> >          """Automatically close the control panel if there is "nothing to
> do"
> >          """
> > diff --git a/src/jarabe/controlpanel/sectionview.py
> b/src/jarabe/controlpanel/sectionview.py
> > index 4b5751f..acd06fc 100644
> > --- a/src/jarabe/controlpanel/sectionview.py
> > +++ b/src/jarabe/controlpanel/sectionview.py
> > @@ -52,3 +52,35 @@ class SectionView(gtk.VBox):
> >      def undo(self):
> >          """Undo here the changes that have been made in this section."""
> >          pass
> > +
> > +    def post_show_section_view(self):
> > +        """
> > +        This method performs actions, after ::
> > +
> > +            1. the initial set-up for the section-view has been set up
> > +               (in '__init__()' of subclasses of
> > +               'jarabe.controlpanel.sectionview.SectionView')
> > +            2. the section-view 'shown' (in 'show_section_view()' of
> > +               'jarabe.controlpanel.gui.ControlPanel')
> > +
> > +        By default, nothing generic needs to be done, as generally, the
> > +        initial set-up is good enough (when no user-interaction is
> > +        required).
> > +
> > +        For a class that actually does something specific, please see
> > +        this over-ridden method in
> > +        'extensions.cpcsection.updater.view.ActivityUpdater'
> > +        """
> > +        pass
> > +
> > +    def post_exit(self):
> > +        """
> > +        This method does the cleaning-up stuff, just as we are about
> > +        to leave the section-view.
> > +
> > +        By default, nothing generic needs to be done.
> > +
> > +        Any subclasses (eg. -
> 'extensions.cpcsection.updater.view.ActivityUpdater'),
> > +        may override this method.
> > +        """
> > +        pass
> > diff --git a/src/jarabe/desktop/homewindow.py
> b/src/jarabe/desktop/homewindow.py
> > index f101757..0bd9d05 100644
> > --- a/src/jarabe/desktop/homewindow.py
> > +++ b/src/jarabe/desktop/homewindow.py
> > @@ -16,6 +16,7 @@
> >
> >  import logging
> >
> > +import os
> >  import gobject
> >  import gtk
> >  import dbus
> > @@ -24,6 +25,7 @@ from gettext import gettext as _
> >  from sugar.graphics import style
> >  from sugar.graphics import palettegroup
> >
> > +from jarabe import config
> >  from jarabe.desktop.meshbox import MeshBox
> >  from jarabe.desktop.homebox import HomeBox
> >  from jarabe.desktop.groupbox import GroupBox
> > @@ -31,6 +33,7 @@ from jarabe.desktop.transitionbox import TransitionBox
> >  from jarabe.model.shell import ShellModel
> >  from jarabe.model import shell
> >  from jarabe.model import notifications
> > +from jarabe.controlpanel.gui import ModelWrapper
> >
> >
> >  _HOME_PAGE = 0
> > @@ -98,6 +101,8 @@ class HomeWindow(gtk.Window):
> >              systembus.add_signal_receiver(self.__relogin_cb, 'Relogin',
> >                                            _DBUS_SYSTEM_IFACE)
> >
> > +        gobject.idle_add(self.load_cpsection_daemon_classes)
> > +
> >      def _system_alert(self, replaces_id, app_icon, message):
> >          service = notifications.get_service()
> >          service.notification_received.send(self,app_name='system',
> > @@ -237,6 +242,36 @@ class HomeWindow(gtk.Window):
> >          self.get_window().set_cursor(gtk.gdk.Cursor(gtk.gdk.WATCH))
> >          gobject.idle_add(action_wrapper, old_cursor)
> >
> > +    def load_cpsection_daemon_classes(self):
> > +        """Get the available option information from the extensions
> > +        """
> > +        options = {}
> > +
> > +        path = os.path.join(config.ext_path, 'cpsection')
> > +        folder = os.listdir(path)
> > +
> > +        for item in folder:
> > +            if os.path.isdir(os.path.join(path, item)) and \
> > +                    os.path.exists(os.path.join(path, item,
> '__init__.py')):
> > +                try:
> > +                    mod = __import__('.'.join(('cpsection', item)),
> > +                                     globals(), locals(), [item])
> > +                    daemon_class = getattr(mod, 'DAEMON', None)
> > +                    if daemon_class is not None:
> > +                        options[item] = {}
> > +                        options[item]['daemon'] = daemon_class
> > +                except Exception:
>
> We should not try to catch all exceptions here, but *only* the ones
> we're expecting to occur.
>

Done.
Changed to except (os.error, TypeError).

Note that 'AttributeError' won't occur during 'getattr', since by default
'None' will be returned.








> > +                    logging.exception('Exception while loading
> extension:')
> > +
> > +
> > +        option_keys = options.keys()
> > +        for key in option_keys:
> > +            mod = __import__('.'.join(('cpsection', key, 'daemon')),
> > +                             globals(), locals(), ['daemon'])
> > +            if mod:
> > +                daemon_class = getattr(mod, options[key]['daemon'],
> None)
> > +                daemon_class()
> > +
> >
> >  def get_instance():
> >      global _instance
>
>
> General comments:
>
> * I've not reviewed the patch very deeply, but I feel we could implement
> this in a cleaner way (with fewer lines of code).
>

Hmm, used the corresponding-method-per-type paradigm.
Looking for a possible better option.







>
> * -1 to another python thread running in the background for this.
>

Well, we require mutual exclusion. If run  in the same main thread, there
are problems of UI freeze, and data corruption during gio.File.read_async
(which reads in  the data from update_url).








>
> * Perhaps we could discuss the ideal solution (in the related [DESIGN]
> thread). Sascha, Aleksey?
>

Looking forward eagerly to hear.







> * Though the comments in-lined in the code were helpful, we could remove
> some of the more verbose ones when committing this to the repo.
>
> Reviewed-by: Anish Mangal <anish at activitycentral.com>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJOeIn2AAoJEBoxUdDHDZVp3BQH/RySJQ3hkkjeN9kH6/ee2kxH
> oy4kDivFjgACk5klPRnBcO+zwvTj49qGlDhCF426LOt8D6uZII5rM0SHzbtb8vEc
> k9pcOK6qhJw6si7prHwh9apt8JPGCif9nxiiWdy5Lse93WiE/pzybI632/8hjLg2
> UzlH5yrK063D0FxQyeOfyxHfcNpeI6wbUTC6IqZI7086YZx5st0IiFI76cXEGZRB
> QM0FWmCbbun5ebhHh3G5epOkPilBqLibtr1wSg3ZiJVwaCXTVVADPUYQ8RVzGtsK
> 9b7v/6aNa6U5fVLfYCSvfgAQaHawCrnUdTDUvw2M2KCSnOeLjL1DQ+E1O6uwxuA=
> =E3Q9
> -----END PGP SIGNATURE-----
>
>
> ------------------------------
>
> Message: 2
> Date: Tue, 20 Sep 2011 11:22:12 -0400
> From: Frederick Grose <fgrose at gmail.com>
> To: dextrose at lists.sugarlabs.org
> Subject: Re: [Dextrose] [PATCH] Resolution for ticket au#887.
> Message-ID:
>        <CAEcBt+XcZsL+ex5e6XN6kkX1-QS5eDY8OSbxV23XOdG4OKH0pg at mail.gmail.com
> >
> Content-Type: text/plain; charset="iso-8859-1"
>
> On Tue, Sep 20, 2011 at 7:52 AM, Ajay Garg <ajaygargnsit at gmail.com> wrote:
>
> > === Usability notes ===
> >
> > 1. Now, besides an interactive 'Software Update', a background
> >   thread also checks for available activity updates. To prevent
> >   possible collisions in a rare situation when the interactive
> >   session and the background thread might run at the same time,
> >   synchronization has been used.
> >
> > 2. When a user enters into 'Software Update' section-view,
> >   exclusive control is gained. The exclusive control is
> >   retained unless and until this section-view is exited
> >   (via 'Cancel' or 'Ok' toolbar buttons).
> >
> >   If the background thread wishes to run, it waits,
> >   while the user interacts with the section-view.
> >
> > 3. If the background thread is running, it has exclusive control.
> >   If the user wishes to enter into an interactive session (by
> >   clicking onto the 'Software Update' section-view-icon),
> >   following happens :
> >
> >   a. The section-view shows 'Getting ready...' markup,
> >      with the message 'Waiting for activity-update-
> >      notifier thread to relinquish control...'
> >
>
> This message would be friendlier to Learners with more general terms, such
> as, 'Waiting for the automatic update check to complete...'
>

Done.
Changed the comment.








>
>     Thanks for the engineering and documentation!
>     --Fred
>

My pleasure, sir.  :) :)







>
> >   b. The (interactive) thread waits, until the background
> >      thread relinquishes control.
> >   c. After the (exclusive) control is retained, the
> >      interactive thread proceeds normally, checking
> >      for updates, followed by user-wish of installing
> >      updates.
> > {...}
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <
> http://lists.sugarlabs.org/archive/dextrose/attachments/20110920/db8a47e2/attachment.html
> >
>
> ------------------------------
>
> _______________________________________________
> Dextrose mailing list
> Dextrose at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/dextrose
>
>
> End of Dextrose Digest, Vol 15, Issue 11
> ****************************************
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20110921/8a0704e1/attachment-0001.html>


More information about the Dextrose mailing list