[Sugar-devel] [PATCH] Journal:show error message on write failure: #1842

[Sascha Silbe]

Excerpts from anishmangal2002's message of Fri Jul 02 19:25:56 +0000 2010:


After this patch, the information flow will be the following:

* model.copy throws *Error 
* BaseButton emits volume-error (strerror)
* VolumesToolbar emits volume-error (strerror)
* JournalActivity instantiates NotifyAlert

This looks a bit convoluted to me, but I don't know whether there's a
better way.


[src/jarabe/journal/journalactivity.py]
> +    def _alert_notify_cb(self, gobject, strerror, severity):
> +        alert = ErrorAlert()
> +        alert.props.title= severity
> +        alert.props.msg = strerror
Please consider passing these directly to the constructor.

> @@ -161,6 +175,8 @@ class JournalActivity(Window):
>          self._volumes_toolbar = VolumesToolbar()
>          self._volumes_toolbar.connect('volume-changed',
>                                        self.__volume_changed_cb)
> +        self._volumes_toolbar.connect('volume-error', self._alert_notify_cb,
> +                                      'Error')
Why do you add an additional parameter ('Error')? AFAICT all signal
handlers should pass through strerror.


[src/jarabe/journal/volumestoolbar.py]
> @@ -137,7 +150,14 @@ class BaseButton(RadioToolButton):
>                                 info, timestamp):
>          object_id = selection_data.data
>          metadata = model.get(object_id)
> -        model.copy(metadata, self.mount_point)
> +        try:
> +            model.copy(metadata, self.mount_point)
> +        except IOError as (errno, strerror):
Please don't use the except ... as ... syntax. It has been introduced in Python 2.6, but we still support 2.5.

> +            logging.error('BaseButton._drag_data_received_cb: IOError: %s; %s' % (errno, strerror))
If you leave the exception intact while catching it (i.e. except IOError, exception:), it will do most of the formatting for you. You might even get below the 80 character line length limit this way. ;)

> +        except ValueError as (strerror):
In what cases does model.copy() throw a ValueError? This sounds more like an internal error we should avoid...
If it does make sense to catch this here, please combine the two except blocks (which will be possible after the logging change mentioned above).

Sascha

--
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
Url : http://lists.sugarlabs.org/archive/sugar-devel/attachments/20100703/6470ad45/attachment.pgp